Skip to content

Commit 04e0512

Browse files
authored
Rollup merge of rust-lang#67015 - osa1:issue66971, r=wesleywiser
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
2 parents 830b4ee + 2404a06 commit 04e0512

File tree

5 files changed

+130
-11
lines changed

5 files changed

+130
-11
lines changed

src/librustc_mir/transform/const_prop.rs

+37-11
Original file line numberDiff line numberDiff line change
@@ -636,19 +636,45 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
636636
ScalarMaybeUndef::Scalar(one),
637637
ScalarMaybeUndef::Scalar(two)
638638
) => {
639+
// Found a value represented as a pair. For now only do cont-prop if type of
640+
// Rvalue is also a pair with two scalars. The more general case is more
641+
// complicated to implement so we'll do it later.
639642
let ty = &value.layout.ty.kind;
643+
// Only do it for tuples
640644
if let ty::Tuple(substs) = ty {
641-
*rval = Rvalue::Aggregate(
642-
Box::new(AggregateKind::Tuple),
643-
vec![
644-
self.operand_from_scalar(
645-
one, substs[0].expect_ty(), source_info.span
646-
),
647-
self.operand_from_scalar(
648-
two, substs[1].expect_ty(), source_info.span
649-
),
650-
],
651-
);
645+
// Only do it if tuple is also a pair with two scalars
646+
if substs.len() == 2 {
647+
let opt_ty1_ty2 = self.use_ecx(source_info, |this| {
648+
let ty1 = substs[0].expect_ty();
649+
let ty2 = substs[1].expect_ty();
650+
let ty_is_scalar = |ty| {
651+
this.ecx
652+
.layout_of(ty)
653+
.ok()
654+
.map(|ty| ty.details.abi.is_scalar())
655+
== Some(true)
656+
};
657+
if ty_is_scalar(ty1) && ty_is_scalar(ty2) {
658+
Ok(Some((ty1, ty2)))
659+
} else {
660+
Ok(None)
661+
}
662+
});
663+
664+
if let Some(Some((ty1, ty2))) = opt_ty1_ty2 {
665+
*rval = Rvalue::Aggregate(
666+
Box::new(AggregateKind::Tuple),
667+
vec![
668+
self.operand_from_scalar(
669+
one, ty1, source_info.span
670+
),
671+
self.operand_from_scalar(
672+
two, ty2, source_info.span
673+
),
674+
],
675+
);
676+
}
677+
}
652678
}
653679
},
654680
_ => { }

src/librustc_target/abi/mod.rs

+8
Original file line numberDiff line numberDiff line change
@@ -802,6 +802,14 @@ impl Abi {
802802
_ => false,
803803
}
804804
}
805+
806+
/// Returns `true` is this is a scalar type
807+
pub fn is_scalar(&self) -> bool {
808+
match *self {
809+
Abi::Scalar(_) => true,
810+
_ => false,
811+
}
812+
}
805813
}
806814

807815
rustc_index::newtype_index! {
+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// compile-flags: -Z mir-opt-level=2
2+
3+
// Due to a bug in propagating scalar pairs the assertion below used to fail. In the expected
4+
// outputs below, after ConstProp this is how _2 would look like with the bug:
5+
//
6+
// _2 = (const Scalar(0x00) : (), const 0u8);
7+
//
8+
// Which has the wrong type.
9+
10+
fn encode(this: ((), u8, u8)) {
11+
assert!(this.2 == 0);
12+
}
13+
14+
fn main() {
15+
encode(((), 0, 0));
16+
}
17+
18+
// END RUST SOURCE
19+
// START rustc.main.ConstProp.before.mir
20+
// bb0: {
21+
// ...
22+
// _3 = ();
23+
// _2 = (move _3, const 0u8, const 0u8);
24+
// ...
25+
// _1 = const encode(move _2) -> bb1;
26+
// ...
27+
// }
28+
// END rustc.main.ConstProp.before.mir
29+
// START rustc.main.ConstProp.after.mir
30+
// bb0: {
31+
// ...
32+
// _3 = const Scalar(<ZST>) : ();
33+
// _2 = (move _3, const 0u8, const 0u8);
34+
// ...
35+
// _1 = const encode(move _2) -> bb1;
36+
// ...
37+
// }
38+
// END rustc.main.ConstProp.after.mir
+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// compile-flags: -Z mir-opt-level=2
2+
3+
// This used to ICE in const-prop
4+
5+
fn test(this: ((u8, u8),)) {
6+
assert!((this.0).0 == 1);
7+
}
8+
9+
fn main() {
10+
test(((1, 2),));
11+
}
12+
13+
// Important bit is parameter passing so we only check that below
14+
// END RUST SOURCE
15+
// START rustc.main.ConstProp.before.mir
16+
// bb0: {
17+
// ...
18+
// _3 = (const 1u8, const 2u8);
19+
// _2 = (move _3,);
20+
// ...
21+
// _1 = const test(move _2) -> bb1;
22+
// ...
23+
// }
24+
// END rustc.main.ConstProp.before.mir
25+
// START rustc.main.ConstProp.after.mir
26+
// bb0: {
27+
// ...
28+
// _3 = (const 1u8, const 2u8);
29+
// _2 = (move _3,);
30+
// ...
31+
// _1 = const test(move _2) -> bb1;
32+
// ...
33+
// }
34+
// END rustc.main.ConstProp.after.mir

src/test/ui/mir/issue66339.rs

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// compile-flags: -Z mir-opt-level=2
2+
// build-pass
3+
4+
// This used to ICE in const-prop
5+
6+
fn foo() {
7+
let bar = |_| { };
8+
let _ = bar("a");
9+
}
10+
11+
fn main() {
12+
foo();
13+
}

0 commit comments

Comments
 (0)