Bug 272705 - benchmarks/unixbench: Remove custom optimization and some minor changes to Makefile
Summary: benchmarks/unixbench: Remove custom optimization and some minor changes to Ma...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Daniel Engberg
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-07-25 00:10 UTC by Daniel Engberg
Modified: 2023-10-02 20:13 UTC (History)
0 users

See Also:
pizzamig: maintainer-feedback+


Attachments
Patch for unixbench (1.41 KB, patch)
2023-07-25 00:10 UTC, Daniel Engberg
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Engberg freebsd_committer freebsd_triage 2023-07-25 00:10:18 UTC
Created attachment 243600 [details]
Patch for unixbench

Define LICENSE_FILE
Use USES= localbase:ldflags
Drop OPTIMIZED_CFLAGS [1]
Do some minor changes to Makefile to improve readability

[1]: Running dhry2reg showed no difference between default optimization compared to custom defined. Whetstone-double showed a difference below 4% so I would call it within margin of error. Using -march=native breaks on various archs and it also overrides CPUTYPE set by framework (if any). Tested on my Tigerlake laptop running 13.2-RELEASE in a VM.

Compile and runtime tested on FreeBSD 13.2-RELEASE (amd64)
Comment 1 Luca Pizzamiglio freebsd_committer freebsd_triage 2023-07-30 15:47:19 UTC
Hi Dizzy.
While there are many cleanups in your patch, that I welcome, I cannot accept this patch as it is.

Instead of removing OPTIMIZED_CFLAGS, I would instead rename it into NATIVE (OFF by default), as it seems more appropriate.

I wouldn't empty the MAKE_ENV, as I don't understand what problem you are trying to solve.
Comment 2 Daniel Engberg freebsd_committer freebsd_triage 2023-07-30 16:14:22 UTC
In #bsdports (IRC) it's been discussed and general consensus is that --march=native is a bad idea especially per port basis and there have been several incidents where it (native) get things wrong and people run into illegal instruction issues. On top of that it's also incompatible with archs that are not amd64 or i386. Additionally we support --march= via CPUTYPE so there's no need trying to work around it.

The MAKE_ENV flags conflicts with framework defaults and this topic is also mentioned in https://docs.freebsd.org/en/books/porters-handbook/book/#dads-cflags . As this is a benchmark application it makes little to no sense overriding the defaults so I don't see a need to manually tune it.

Best regards,
Daniel
Comment 3 Daniel Engberg freebsd_committer freebsd_triage 2023-07-30 16:18:05 UTC
Forgot to mention, upstream's Makefile sets -O3 -ffast-math if UB_GCC_OPTIONS is undefined.

https://github.com/kdlucas/byte-unixbench/blob/master/UnixBench/Makefile#L62
Comment 4 Daniel Engberg freebsd_committer freebsd_triage 2023-08-27 15:55:43 UTC
Friendly ping
Comment 5 Luca Pizzamiglio freebsd_committer freebsd_triage 2023-09-28 19:04:11 UTC
Hi Daniel.
Feel free to commit it as it is.

At a later point, I could introduce the NATIVE option (OFF by default, ofc), if it makes sense.
Comment 6 commit-hook freebsd_committer freebsd_triage 2023-10-02 19:50:32 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=d1266382c292f5f6731f0e29591cabbbd13f9f93

commit d1266382c292f5f6731f0e29591cabbbd13f9f93
Author:     Daniel Engberg <diizzy@FreeBSD.org>
AuthorDate: 2023-09-30 21:32:02 +0000
Commit:     Daniel Engberg <diizzy@FreeBSD.org>
CommitDate: 2023-10-02 19:50:13 +0000

    benchmarks/unixbench: Remove custom optimization and clean up Makefile

    * Define LICENSE_FILE
    * Use USES= localbase:ldflags helper
    * Drop OPTIMIZED_CFLAGS
    * Some changes to Makefile to improve readability

    PR:             272705
    Reviewed by:    pizzamig (maintainer)

 benchmarks/unixbench/Makefile | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)
Comment 7 Daniel Engberg freebsd_committer freebsd_triage 2023-10-02 20:13:31 UTC
Committed, thanks