Stream: t-compiler/wg-prioritization/alerts

Topic: I-prioritize #76387 Optimisation-caused UB during cross-cra…


triagebot (Sep 05 2020 at 22:16, on Zulip):

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

Procedure

Joshua Nelson (Sep 05 2020 at 22:47, on Zulip):

I'd say P-high? Looks pretty bad but they haven't said it's a regression

Joshua Nelson (Sep 05 2020 at 23:05, on Zulip):

yup, confirmed not a regression

LeSeulArtichaut (Sep 06 2020 at 08:50, on Zulip):

Let’s ping LLVM group for this?

Joshua Nelson (Sep 06 2020 at 11:45, on Zulip):

I'm not sure it's LLVM

Joshua Nelson (Sep 06 2020 at 11:45, on Zulip):

It sounded like a MIR opt gone bad

Joshua Nelson (Sep 06 2020 at 11:46, on Zulip):

Oh wait they said

Dead Argument Elimination

So yeah, LLVM

Joshua Nelson (Sep 06 2020 at 11:46, on Zulip):

Would be nice to have a reproduction in C then

LeSeulArtichaut (Sep 06 2020 at 12:03, on Zulip):

+1 for P-high

triagebot (Sep 06 2020 at 12:08, on Zulip):

Issue #76387's prioritization request has been removed.

Santiago Pastorino (Sep 09 2020 at 18:51, on Zulip):

@LeSeulArtichaut @Joshua Nelson I've just seen that you have agreed on P-high but it seems P-critical to me

Joshua Nelson (Sep 09 2020 at 18:53, on Zulip):

sounds fine to me, @nagisa also suggested P-critical

Santiago Pastorino (Sep 09 2020 at 18:53, on Zulip):

I understand that this is an LLVM issue and maybe that make us lower a bit the priority, on the other hand I think I'd rather tag this kind of issues as P-critical as in the end they affect Rust very badly and there's something we can do, like backing out the LLVM 11 upgrade if needed

Santiago Pastorino (Sep 09 2020 at 18:55, on Zulip):

anyway, I wanted to discuss this :point_up: briefly to see what others do and think, my opinion is that I wouldn't lower the priority of an issue because it's LLVM or a Rust 3rd party dependency because they still affect Rust and almost always there's something we can do :)

Joshua Nelson (Sep 09 2020 at 18:56, on Zulip):

my reasoning originally is that no one had come across it for 5 years, but if it's unsound it should probably higher yeah

Santiago Pastorino (Sep 09 2020 at 18:56, on Zulip):

ok :)

Santiago Pastorino (Sep 09 2020 at 18:56, on Zulip):

but in general, what do you think about my thoughts?

Santiago Pastorino (Sep 09 2020 at 18:56, on Zulip):

regardless of this particular issue

Joshua Nelson (Sep 09 2020 at 19:02, on Zulip):

makes sense :)

Santiago Pastorino (Sep 09 2020 at 19:05, on Zulip):

anyway, back to this particular case, I'm kind of having second thoughts about the priority of it :)

Santiago Pastorino (Sep 09 2020 at 19:05, on Zulip):

was checking and yeah this issue exists since a TON

Santiago Pastorino (Sep 09 2020 at 19:05, on Zulip):

in general I tend to think that soundness issues are always P-critical but ... unsure ...

Santiago Pastorino (Sep 09 2020 at 19:06, on Zulip):

cc @pnkfelix who might want to also weigh in

apiraino (Sep 09 2020 at 19:11, on Zulip):

Can we assign the best label based on our estimate, then tomorrow during the meeting it will be discussed anyway and possibly the priorty adjusted?

Santiago Pastorino (Sep 09 2020 at 20:10, on Zulip):

yes, that would be ok

Santiago Pastorino (Sep 09 2020 at 20:10, on Zulip):

but just in case, I've already labelled as P-critical

Santiago Pastorino (Sep 09 2020 at 20:11, on Zulip):

just mentioned Felix in case they wanted to give an opinion

apiraino (Sep 14 2020 at 20:52, on Zulip):

@Santiago Pastorino According to last week's meetings notes, the issue is being actively worked on, can we remove the I-prio label?
https://rust-lang.zulipchat.com/#narrow/stream/238009-t-compiler.2Fmeetings/topic/.5Bweekly.20meeting.5D.202020-09-10.20.2354818/near/209658278

Santiago Pastorino (Sep 14 2020 at 21:00, on Zulip):

there's no I-prioritize label in this issue

Santiago Pastorino (Sep 14 2020 at 21:00, on Zulip):

unsure what did you mean exactly

apiraino (Sep 14 2020 at 21:07, on Zulip):

ignore me

LeSeulArtichaut (Oct 22 2020 at 15:19, on Zulip):

@Aaron Hill @pnkfelix (if you want to discuss here for LLVM cherry-pick)

Santiago Pastorino (Oct 22 2020 at 21:55, on Zulip):

to backport I guess the commit should be cherry-picked to https://github.com/rust-lang/llvm-project and then on the rust repo just update the llvm-project submodule

Santiago Pastorino (Oct 22 2020 at 21:55, on Zulip):

example https://github.com/rust-lang/llvm-project/pull/81

Santiago Pastorino (Oct 22 2020 at 21:55, on Zulip):

cc @Aaron Hill

Aaron Hill (Oct 22 2020 at 22:00, on Zulip):

I'm writing a regression test, and I'll open a rustc pr bumping llvm soon

Aaron Hill (Oct 22 2020 at 22:17, on Zulip):

Opened https://github.com/rust-lang/rust/pull/78253

Last update: Apr 10 2021 at 21:30UTC