Bug 278155 - math/libtommath: Fix plist and some minor improvements
Summary: math/libtommath: Fix plist and some minor improvements
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: Pietro Cerutti
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-04-03 22:18 UTC by Daniel Engberg
Modified: 2024-04-06 12:22 UTC (History)
0 users

See Also:
bugzilla: maintainer-feedback? (gahr)

Attachments
Patch for libtommath (3.65 KB, patch)
2024-04-03 22:18 UTC, Daniel Engberg 		no flags Details | Diff
Patch for libtommath v2 (5.24 KB, patch)
2024-04-05 17:47 UTC, Daniel Engberg 		no flags Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Engberg freebsd_committer freebsd_triage 2024-04-03 22:18:18 UTC 
Created attachment 249688 [details]
Patch for libtommath

- Use framework helpers
- Fix plist
- Disable ccache "detection"
- Don't override framework CFLAGS, make upstreams suggested optimization optional

See also:
https://docs.freebsd.org/en/books/porters-handbook/book/#dads-cflags
Comment 1 Pietro Cerutti freebsd_committer freebsd_triage 2024-04-04 04:08:26 UTC 
The broken plist was due to a patch file I forgot to add. Fixed in edab8726992480217bcef72d40947150d6.

Can you please rebase your patch? I'll think about your other suggestions.
Comment 2 Pietro Cerutti freebsd_committer freebsd_triage 2024-04-04 06:49:14 UTC 
How about we merge LTO and optimizations in one single option?

+OPTIONS_DEFINE=        OPTIMIZED_CFLAGS
+OPTIMIZED_CFLAGS_CFLAGS=       -O3 -funroll-loops -fomit-frame-pointer
+OPTIMIZED_CFLAGS_CMAKE_BOOL=   COMPILE_LTO
Comment 3 Pietro Cerutti freebsd_committer freebsd_triage 2024-04-04 06:49:48 UTC 
What's wrong with the automatic ccache detection?
Comment 4 Daniel Engberg freebsd_committer freebsd_triage 2024-04-05 17:22:46 UTC 
I'd rather see that we don't come up with new options since both are already establied in the tree.

https://cgit.freebsd.org/ports/tree/Mk/bsd.options.desc.mk#n397
https://cgit.freebsd.org/ports/tree/Mk/bsd.options.desc.mk#n320

The ccache logic blindly assumes that if ccache happens to be installed that it should use it. It's going away in https://github.com/libtom/libtommath/pull/577
Comment 5 Daniel Engberg freebsd_committer freebsd_triage 2024-04-05 17:47:52 UTC 
Created attachment 249743 [details]
Patch for libtommath v2

Rebase, make LTO option default (seems to perform as good as -O3 etc) and tests runs fine
Comment 6 Pietro Cerutti freebsd_committer freebsd_triage 2024-04-06 05:33:36 UTC 
Ok, I'll put OPTIMIZED_CFLAGS in the defaults too, though.
Comment 7 Daniel Engberg freebsd_committer freebsd_triage 2024-04-06 08:16:23 UTC 
Have you tested these on mutiple platforms? Doing some small tests on amd64 and aarch64 (VMs) suggests that LTO yields about the same performance on its own with stock CFLAGS.
Comment 8 commit-hook freebsd_committer freebsd_triage 2024-04-06 12:21:21 UTC 
A commit in branch main references this bug:

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

commit 8509bb6725e6252f5a9bc193b57a83f4843b0e04
Author:     Pietro Cerutti <gahr@FreeBSD.org>
AuthorDate: 2024-04-06 12:18:31 +0000
Commit:     Pietro Cerutti <gahr@FreeBSD.org>
CommitDate: 2024-04-06 12:21:06 +0000

    math/libtommath: small improvements

    - remove built-in ccache detection, see
      https://github.com/libtom/libtommath/pull/577/
    - put OPTIMIZED_CFLAGS and LTO in OPTIONS, on by default

    PR:             278155
    Reported by:    diizzy

 math/libtommath/Makefile                   |  7 +++++++
 math/libtommath/files/patch-CMakeLists.txt | 15 +++++++++++++++
 2 files changed, 22 insertions(+)
Comment 9 Pietro Cerutti freebsd_committer freebsd_triage 2024-04-06 12:22:12 UTC 
Now they're both in OPTIONS. Feel free to tweak your build as you see fit. Thanks!