Closed
Bug 1469088
Opened 6 years ago
Closed 6 years ago
--enable-lto is too opinionated
Categories
(Firefox Build System :: General, enhancement)
Tracking
(firefox63 fixed)
RESOLVED
FIXED
mozilla63
| Tracking | Status | |
|---|---|---|
| firefox63 | --- | fixed |
People
(Reporter: glandium, Assigned: glandium)
References
Details
Attachments
(1 file)
|
(deleted),
text/x-review-board-request
|
Details |
The code added in bug 1457482 is too opinionated, forcing -fuse-ld=lld when the compiler is clang, and implying --enable-linker=lld. This effectively prevents from doing lto on mac cross builds using the option (which is more desirable than setting C/CXXFLAGS)
|
Assignee | |
Updated•6 years ago
|
Assignee: nobody → mh+mozilla
| Comment hidden (mozreview-request) |
|
||
Comment 2•6 years ago
|
||
| mozreview-review | ||
Comment on attachment 8989902 [details]
Bug 1469088 - Relax the assumptions of --enable-lto, and make it work for macOS builds.
https://reviewboard.mozilla.org/r/254898/#review262300
::: commit-message-fbb6d:15
(Diff revision 1)
> +to:
> +- work on macOS, which it currently doesn't, and add support for the mac
> + linker (ld64), which, unfortunately, doesn't have a clean way to be
> + detected, so work around that.
> +- default to lld if no linker was explicitly given and the compiler is
> + clang *and* the build target is not macOS.
This doesn't seem to describe what you've implemented precisely, in that your conditional is these three things *and* lto is enabled.
::: build/moz.configure/toolchain.configure:1633
(Diff revision 1)
> - cmd_output = check_cmd_output(*cmd, onerror=lambda: None)
> - if cmd_output is None:
> + # ld64 doesn't have anything to print out a version. It does print out
> + # "ld64: For information on command line options please use 'man ld'."
> + # but that would require doing two attempts, one with --version, that
> + # would fail, and another with --help.
> + # Instead, abuse its LD_PRINT_OPTIONS feature to detect a message
> + # specific to it on stderr when it fails to process --version.
This is a clever hack. :)
Attachment #8989902 -
Flags: review+
|
|
||
Updated•6 years ago
|
Attachment #8989902 -
Flags: review?(core-build-config-reviews)
| Comment hidden (mozreview-request) |
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/e8c173a632a4
Relax the assumptions of --enable-lto, and make it work for macOS builds. r=ted
|
|
Assignee | |
Updated•6 years ago
|
Attachment #8989902 -
Flags: review?(core-build-config-reviews)
|
||
Comment 5•6 years ago
|
||
| bugherder | ||
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
|
||
Updated•6 years ago
|
Version: Version 3 → 3 Branch
You need to log in
before you can comment on or make changes to this bug.
Description
•