Stream: t-compiler/wg-prioritization/alerts

Topic: I-prioritize #74657 linker-plugin-lto stopped working in Rus


triagebot (Jul 22 2020 at 21:45, on Zulip):

@WG-prioritization/alerts issue #74657 has been requested for prioritization.

Procedure

Santiago Pastorino (Jul 29 2020 at 15:31, on Zulip):

this went unnoticed, taking a look at it right now

Santiago Pastorino (Jul 29 2020 at 15:31, on Zulip):

and is the last one lacking prioritization :)

lcnr (Jul 29 2020 at 15:33, on Zulip):

hmm, P-high maybe :thinking:

Santiago Pastorino (Jul 29 2020 at 15:33, on Zulip):

yeah I was wondering the same

lcnr (Jul 29 2020 at 15:33, on Zulip):

I think we need a mvce here though

Santiago Pastorino (Jul 29 2020 at 15:33, on Zulip):

but I'm right now thinking loudly, why not P-critical?

Santiago Pastorino (Jul 29 2020 at 15:34, on Zulip):

I'm trying to think, what kind of peculiarity this have that is uncommon

Santiago Pastorino (Jul 29 2020 at 15:34, on Zulip):

my first reaction was, if Firefox doesn't compile then P-critical

Santiago Pastorino (Jul 29 2020 at 15:34, on Zulip):

:)

lcnr (Jul 29 2020 at 15:35, on Zulip):

RUSTFLAGS="-Clinker-plugin-lto=LLVMgold.so" and only on some target apparently

Santiago Pastorino (Jul 29 2020 at 15:36, on Zulip):

ahh ok

Santiago Pastorino (Jul 29 2020 at 15:36, on Zulip):

yeah saw the flags, but didn't realize about the targets

lcnr (Jul 29 2020 at 15:36, on Zulip):

so most firefox targets still work here

lcnr (Jul 29 2020 at 15:36, on Zulip):

at least from what I can tell it would be quite bad if this weren't the case

LeSeulArtichaut (Jul 29 2020 at 15:37, on Zulip):

I guess this was fixed on Firefox’s side, see the linkrf bugzilla issue

Santiago Pastorino (Jul 29 2020 at 15:39, on Zulip):

yeah they worked around the issue

Santiago Pastorino (Jul 29 2020 at 15:40, on Zulip):

going with P-high then

Santiago Pastorino (Jul 29 2020 at 15:41, on Zulip):

this even hit archlinux firefox builds

Santiago Pastorino (Jul 29 2020 at 15:41, on Zulip):

or am I wrong?

Santiago Pastorino (Jul 29 2020 at 15:41, on Zulip):

https://github.com/archlinux/svntogit-packages/commit/7d760401eaef0b80514d2c87b0bfd1f0fb35cc30

lcnr (Jul 29 2020 at 15:44, on Zulip):

wasn't that fix for a build perf regression?

lcnr (Jul 29 2020 at 15:44, on Zulip):

:shock:

lcnr (Jul 29 2020 at 15:44, on Zulip):

https://bugzilla.mozilla.org/show_bug.cgi?id=1654465

Santiago Pastorino (Jul 29 2020 at 15:44, on Zulip):

to be honest unsure

Santiago Pastorino (Jul 29 2020 at 15:45, on Zulip):

too long didn't properly read :)

Santiago Pastorino (Jul 29 2020 at 15:45, on Zulip):

@lcnr thoughts about what to do here?

lcnr (Jul 29 2020 at 15:46, on Zulip):

ping ICE breakers and hope they are able to somehow minimize this :sweat_smile:

This seems like an annoying bug to track down

Santiago Pastorino (Jul 29 2020 at 15:46, on Zulip):

hehe, but should we start with P-high then?

lcnr (Jul 29 2020 at 15:47, on Zulip):

A comment in the that thread mentions that some args to rustc changed with that version

lcnr (Jul 29 2020 at 15:48, on Zulip):
Since the original problem is that cargo >= 1.45 passes extra flags (-C embed-bitcode=no)
to rustc that are incompatible with -Clto, and while
it knows to adjust based on the lto setting in the build profile
(which CARGO_PROFILE_RELEASE_LTO overrides the default of), cargo
ignores flags passed via cargo rustc -- ... when making those
adjustments.

So, we need to override with -C embed-bitcode=yes on our own at the
same time we pass -Clto. But doing that through cargo rustc -- ...
is not enough because all the dependencies of the crate built with
-Clto need to be built with -C embed-bitcode=yes. So we need to
override with RUSTFLAGS, which will affect all the dependencies.
But we also need to do this consistently across all crates, not only the
dependencies of crates built with -Clto, otherwise we'd still end up
building crates twice (once with and once without the override).
lcnr (Jul 29 2020 at 15:49, on Zulip):

maybe :/ from what I can see the issue isn't really P-critical, but I also don't know what causes it

Santiago Pastorino (Jul 29 2020 at 15:49, on Zulip):

hmm right

Santiago Pastorino (Jul 29 2020 at 15:50, on Zulip):

I guess I'd go with P-high until someone figures out something that gives more light

lcnr (Jul 29 2020 at 15:50, on Zulip):

we should also ping cleanup here I think

lcnr (Jul 29 2020 at 15:50, on Zulip):

if someone wants to spend a few hours building firefox

Santiago Pastorino (Jul 29 2020 at 15:50, on Zulip):

lol

Santiago Pastorino (Jul 29 2020 at 15:51, on Zulip):

right, I've pinged them

Santiago Pastorino (Jul 29 2020 at 15:51, on Zulip):

going with P-highfor now then

LeSeulArtichaut (Jul 29 2020 at 15:52, on Zulip):

IIRC this was also reproducible with rusqlite?

lcnr (Jul 29 2020 at 15:55, on Zulip):

hmm, yeah. Looks like the issue was with rusqlite...

lcnr (Jul 29 2020 at 15:55, on Zulip):

so maybe it's not "if someone wants to spend a few hours building firefox" bad

LeSeulArtichaut (Jul 29 2020 at 16:00, on Zulip):

We already have the bisection though

LeSeulArtichaut (Jul 29 2020 at 16:00, on Zulip):

https://github.com/rust-lang/rust/commit/1357af3a55e24c7d28abb5eda258662f2307fadd

lcnr (Jul 29 2020 at 16:00, on Zulip):

but no mvce, if I didn't miss something here

LeSeulArtichaut (Jul 29 2020 at 16:00, on Zulip):

Right

LeSeulArtichaut (Jul 29 2020 at 16:01, on Zulip):

We should ping Alex Crichton I think

lcnr (Jul 29 2020 at 16:01, on Zulip):

hmm, yeah... might help

LeSeulArtichaut (Jul 29 2020 at 16:01, on Zulip):

You do it or shall I?

lcnr (Jul 29 2020 at 16:02, on Zulip):

feel free, I should actually be working for uni rn :sweat_smile:

Last update: Apr 17 2021 at 00:15UTC