Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix constant propagation for scalar pairs #67015

Merged
merged 1 commit into from
Dec 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 37 additions & 11 deletions src/librustc_mir/transform/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -636,19 +636,45 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
ScalarMaybeUndef::Scalar(one),
ScalarMaybeUndef::Scalar(two)
) => {
// Found a value represented as a pair. For now only do cont-prop if type of
// Rvalue is also a pair with two scalars. The more general case is more
// complicated to implement so we'll do it later.
let ty = &value.layout.ty.kind;
// Only do it for tuples
if let ty::Tuple(substs) = ty {
*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
),
],
);
// Only do it if tuple is also a pair with two scalars
if substs.len() == 2 {
let opt_ty1_ty2 = self.use_ecx(source_info, |this| {
let ty1 = substs[0].expect_ty();
let ty2 = substs[1].expect_ty();
let ty_is_scalar = |ty| {
this.ecx
.layout_of(ty)
.ok()
.map(|ty| ty.details.abi.is_scalar())
== Some(true)
};
if ty_is_scalar(ty1) && ty_is_scalar(ty2) {
Ok(Some((ty1, ty2)))
} else {
Ok(None)
}
});

if let Some(Some((ty1, ty2))) = opt_ty1_ty2 {
*rval = Rvalue::Aggregate(
Box::new(AggregateKind::Tuple),
vec![
self.operand_from_scalar(
one, ty1, source_info.span
),
self.operand_from_scalar(
two, ty2, source_info.span
),
],
);
}
}
}
},
_ => { }
Expand Down
8 changes: 8 additions & 0 deletions src/librustc_target/abi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,14 @@ impl Abi {
_ => false,
}
}

/// Returns `true` is this is a scalar type
pub fn is_scalar(&self) -> bool {
match *self {
Abi::Scalar(_) => true,
_ => false,
}
}
}

rustc_index::newtype_index! {
Expand Down
38 changes: 38 additions & 0 deletions src/test/mir-opt/const_prop/issue-66971.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// compile-flags: -Z mir-opt-level=2

// Due to a bug in propagating scalar pairs the assertion below used to fail. In the expected
// outputs below, after ConstProp this is how _2 would look like with the bug:
//
// _2 = (const Scalar(0x00) : (), const 0u8);
//
// Which has the wrong type.

fn encode(this: ((), u8, u8)) {
assert!(this.2 == 0);
}

fn main() {
encode(((), 0, 0));
}

// END RUST SOURCE
// START rustc.main.ConstProp.before.mir
// bb0: {
// ...
// _3 = ();
// _2 = (move _3, const 0u8, const 0u8);
// ...
// _1 = const encode(move _2) -> bb1;
// ...
// }
// END rustc.main.ConstProp.before.mir
// START rustc.main.ConstProp.after.mir
// bb0: {
// ...
// _3 = const Scalar(<ZST>) : ();
// _2 = (move _3, const 0u8, const 0u8);
// ...
// _1 = const encode(move _2) -> bb1;
// ...
// }
// END rustc.main.ConstProp.after.mir
34 changes: 34 additions & 0 deletions src/test/mir-opt/const_prop/issue-67019.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// compile-flags: -Z mir-opt-level=2

// This used to ICE in const-prop

fn test(this: ((u8, u8),)) {
assert!((this.0).0 == 1);
}

fn main() {
test(((1, 2),));
}

// Important bit is parameter passing so we only check that below
// END RUST SOURCE
// START rustc.main.ConstProp.before.mir
// bb0: {
// ...
// _3 = (const 1u8, const 2u8);
// _2 = (move _3,);
// ...
// _1 = const test(move _2) -> bb1;
// ...
// }
// END rustc.main.ConstProp.before.mir
// START rustc.main.ConstProp.after.mir
// bb0: {
// ...
// _3 = (const 1u8, const 2u8);
// _2 = (move _3,);
// ...
// _1 = const test(move _2) -> bb1;
// ...
// }
// END rustc.main.ConstProp.after.mir
13 changes: 13 additions & 0 deletions src/test/ui/mir/issue66339.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// compile-flags: -Z mir-opt-level=2
// build-pass

// This used to ICE in const-prop

fn foo() {
let bar = |_| { };
let _ = bar("a");
}

fn main() {
foo();
}