-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Miscompilation of tuple arguments with mir-opt-level=2 #66971
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Comments
Slightly simpler reproducer fails the same way:
|
I may be talking nonsense (first time looking at a mir dump), but I think in
|
Looking at MIR dumps, before constant propagation this is how
After:
So the last field disappears. |
I think the bug is in these lines: fn replace_with_const(
&mut self,
rval: &mut Rvalue<'tcx>,
value: Const<'tcx>,
source_info: SourceInfo,
) {
...
Immediate::ScalarPair(
ScalarMaybeUndef::Scalar(one),
ScalarMaybeUndef::Scalar(two)
) => {
let ty = &value.layout.ty.kind;
if let ty::Tuple(substs) = ty {
debug!("substs: {:#?}", substs);
*rval = Rvalue::Aggregate(
Box::new(AggregateKind::Tuple),
vec![
self.operand_from_scalar(
one, substs[0].expect_ty(), source_info.span
),
self.operand_from_scalar(
two, substs[1].expect_ty(), source_info.span
),
],
);
}
... In this program when visiting |
Confirmed that commenting out this block of code fixes the issue. Not sure how to proceed. If we want to do this optimization for scalar pairs |
Since we are making the decision on something that came from a layout, but then use the type to get at said field, we should probably figure out a layout based scheme for this. You can ask the layout for a field count, then iterate and use the |
Thanks @oli-obk. I have a fix, PR coming. |
Fix constant propagation for scalar pairs We now only propagate a scalar pair if the Rvalue is a tuple with two scalars. This for example avoids propagating a (u8, u8) value when Rvalue has type `((), u8, u8)` (see the regression test). While this is a correct thing to do, implementation is tricky and will be done later. Fixes rust-lang#66971 Fixes rust-lang#66339 Fixes rust-lang#67019
This reduced testcase is getting miscompiled with
mir-opt-level=2
. I bisected it to #64890.Rust version:
The text was updated successfully, but these errors were encountered: