Android linkage failure with LTO enabled

RESOLVED FIXED in Firefox 63

Status

defect
RESOLVED FIXED
11 months ago
10 months ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

unspecified
mozilla63
Dependency tree / graph

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

11 months ago
When enabling LTO on Android, the build fails with:

ld.lld: error: --plugin-opt: number expected, but got 'z'

The problem is that LLVM doesn't like -Oz or -Os passed to the linker for LTO. It *does* respect the -Oz/-Os passed during compiler calls, but it needs e.g. -O2 to be passed at link time.

I've used a gross hack on try: wrap calls to EXPAND_CC_OR_CXX and MKSHLIB with $(subst -Oz,-O2,...) in config/rules.mk, but that is not friendly to other build backends.

The build system doesn't make a distinction between compile-optimization flags and link-optimization flags.

Question is, what would be a good way to handle this?
Flags: needinfo?(mshal)
Flags: needinfo?(cmanchester)
Assignee

Updated

11 months ago
Blocks: android-lto
Compile optimize flags are only passed to the linker because of "C_LDFLAGS" and "CXX_LDFLAGS" here: https://searchfox.org/mozilla-central/rev/196560b95f191b48ff7cba7c2ba9237bba6b5b6a/python/mozbuild/mozbuild/frontend/context.py#470

Maybe we can remove those and have configure decide whether to pass MOZ_OPTIMIZE_FLAGS or something a bit different into MOZ_OPTIMIZE_LDFLAGS, which are only passed to the linker. We'd probably have to move the logic around MOZ_PGO_OPTIMIZE_FLAGS somewhere the linker flags could take advantage of as well.
Flags: needinfo?(cmanchester)
I defer to chmanchester on flags handling, but I'll also note that tup or other alternate backends for Android are likely a ways away. So if this is an Android-specific issue, it's fine IMO if we need to resort to a rules.mk hack for now.
Flags: needinfo?(mshal)
Assignee

Comment 3

10 months ago
Assignee: nobody → mh+mozilla
Attachment #8999092 - Flags: review?(core-build-config-reviews)
Comment on attachment 8999092 [details] [diff] [review]
Normalize optimization level passed to the linker when doing LTO with clang

Review of attachment 8999092 [details] [diff] [review]:
-----------------------------------------------------------------

I am sort of willing to believe that this would do the trick.  Just to be clear, what this patch does is that we were passing:

 clang++ ... $MOZ_OPTIMIZE_FLAGS $MOZ_OPTIMIZE_LDFLAGS

and those flags all get parsed, then handed off to the linker, which was getting the -Os/-Oz from $MOZ_OPTIMIZE_FLAGS?  But now, the -O2 from $MOZ_OPTIMIZE_LDFLAGS takes precedence, so when the parsed flags are handed off to the linker, the -O2 overrides the -Os/-Oz from $MOZ_OPTIMIZE_FLAGS, and everything works out?

That feels pretty subtle.

::: old-configure.in
@@ +3475,5 @@
> +    if test -n "$MOZ_LTO" -a -n "$CLANG_CC"; then
> +        # When using llvm-based LTO, non numeric optimization levels are
> +        # not supported by the linker, so force the linker to use -O2 (
> +        # which doesn't influence the level compilation units are actually
> +        # compiled at).

i.e. This comment is saying that -O2 is just there to make the linker happy, and the original optimization flags are recorded in the object files or something, and that's what actually gets used?  Or something else?
Assignee

Comment 5

10 months ago
The optimization level given to the linker apparently(*) is only used to drive what the linker itself generates (like, trampoline and some such), but codegen for compilation units still follows what was given to the compiler. If you think of the compiler part doing the equivalent of `#pragma optimize` on all the code compiled, it makes sense.

And yes, this relies on the fact that -Os -O2 is interpreted as -O2 by the linker. It's subtle, but if things end up in the wrong order, it's quite visible, since that fails to link.
Comment on attachment 8999092 [details] [diff] [review]
Normalize optimization level passed to the linker when doing LTO with clang

Review of attachment 8999092 [details] [diff] [review]:
-----------------------------------------------------------------

OK, thanks for confirming that.
Attachment #8999092 - Flags: review?(core-build-config-reviews) → review+
Assignee

Comment 7

10 months ago
(In reply to Mike Hommey [:glandium] from comment #5)
> The optimization level given to the linker apparently(*)

Forgot to fill the note:
* Compiling with -Oz and linking with -O2 generates smaller APKs than compiling with -O2 and linking with -O2.

Comment 8

10 months ago
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/783e7599b5c2
Normalize optimization level passed to the linker when doing LTO with clang. r=froydnj

Comment 9

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/783e7599b5c2
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.