about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMazdak Farrokhzad <twingoow@gmail.com>2019-12-11 10:10:42 +0100
committerGitHub <noreply@github.com>2019-12-11 10:10:42 +0100
commit04e0512c7a94736e104e5d25a4705c37fa93a7e2 (patch)
tree4c296527ca67b7f6b05c7465a58372d9eac431a5
parent830b4ee76adea7577615c7a6950e14c22cf0fb20 (diff)
parent2404a067eedd83ab69bb0e07fdc8145825741722 (diff)
downloadrust-04e0512c7a94736e104e5d25a4705c37fa93a7e2.tar.gz
rust-04e0512c7a94736e104e5d25a4705c37fa93a7e2.zip
Rollup merge of #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 #66971
Fixes #66339
Fixes #67019
-rw-r--r--src/librustc_mir/transform/const_prop.rs48
-rw-r--r--src/librustc_target/abi/mod.rs8
-rw-r--r--src/test/mir-opt/const_prop/issue-66971.rs38
-rw-r--r--src/test/mir-opt/const_prop/issue-67019.rs34
-rw-r--r--src/test/ui/mir/issue66339.rs13
5 files changed, 130 insertions, 11 deletions
diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs
index 884312514e4..aff91ac5af9 100644
--- a/src/librustc_mir/transform/const_prop.rs
+++ b/src/librustc_mir/transform/const_prop.rs
@@ -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
+                                        ),
+                                    ],
+                                );
+                            }
+                        }
                     }
                 },
                 _ => { }
diff --git a/src/librustc_target/abi/mod.rs b/src/librustc_target/abi/mod.rs
index ac781819cc3..b0bc0529297 100644
--- a/src/librustc_target/abi/mod.rs
+++ b/src/librustc_target/abi/mod.rs
@@ -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! {
diff --git a/src/test/mir-opt/const_prop/issue-66971.rs b/src/test/mir-opt/const_prop/issue-66971.rs
new file mode 100644
index 00000000000..30c75303b3e
--- /dev/null
+++ b/src/test/mir-opt/const_prop/issue-66971.rs
@@ -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
diff --git a/src/test/mir-opt/const_prop/issue-67019.rs b/src/test/mir-opt/const_prop/issue-67019.rs
new file mode 100644
index 00000000000..c6d753a1209
--- /dev/null
+++ b/src/test/mir-opt/const_prop/issue-67019.rs
@@ -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
diff --git a/src/test/ui/mir/issue66339.rs b/src/test/ui/mir/issue66339.rs
new file mode 100644
index 00000000000..98e178c0551
--- /dev/null
+++ b/src/test/ui/mir/issue66339.rs
@@ -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();
+}