about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2020-09-01 13:36:52 +0000
committerbors <bors@rust-lang.org>2020-09-01 13:36:52 +0000
commit397db054cb1f3d98e3d2809d25c60f1979cd5a97 (patch)
treeacc9eb26a2489bbe4adf478d0f50020f7040ba64
parente88e908e66cd1e6e30d789b37bcd774951d01856 (diff)
parent1d157ce797dddcee16a577796199b1144b4f7f34 (diff)
downloadrust-397db054cb1f3d98e3d2809d25c60f1979cd5a97.tar.gz
rust-397db054cb1f3d98e3d2809d25c60f1979cd5a97.zip
Auto merge of #75529 - bugadani:bounds-check, r=nagisa
Eliminate some other bound checks when index comes from an enum

#36962 introduced an assumption for the upper limit of the enum's value. This PR adds an assumption to the lower value as well.

I've modified the original codegen test to show that derived (in that case, adding 1) values also don't generate bounds checks.

However, this test is actually carefully crafted to not hit a bug: if the enum's variants are modified to 1 and 2 instead of 2 and 3, the test fails by adding a bounds check. I suppose this is an LLVM issue and #75525, while not exactly in this context should be tracking it.

I'm not at all confident if this patch can be accepted, or even if it _should_ be accepted in this state. But I'm curious about what others think :)

~Improves~ Should improve #13926 but does not close it because it's not exactly predictable, where bounds checks may pop up against the assumptions.
-rw-r--r--compiler/rustc_codegen_ssa/src/mir/rvalue.rs26
-rw-r--r--src/test/codegen/enum-bounds-check-derived-idx.rs25
-rw-r--r--src/test/codegen/enum-bounds-check-issue-13926.rs19
-rw-r--r--src/test/codegen/enum-bounds-check.rs12
4 files changed, 77 insertions, 5 deletions
diff --git a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs
index 1994c45707c..c64fc9bfcf0 100644
--- a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs
+++ b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs
@@ -327,13 +327,29 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                                 if er.end != er.start
                                     && scalar.valid_range.end() > scalar.valid_range.start()
                                 {
-                                    // We want `table[e as usize]` to not
+                                    // We want `table[e as usize ± k]` to not
                                     // have bound checks, and this is the most
-                                    // convenient place to put the `assume`.
-                                    let ll_t_in_const =
+                                    // convenient place to put the `assume`s.
+                                    if *scalar.valid_range.start() > 0 {
+                                        let enum_value_lower_bound = bx
+                                            .cx()
+                                            .const_uint_big(ll_t_in, *scalar.valid_range.start());
+                                        let cmp_start = bx.icmp(
+                                            IntPredicate::IntUGE,
+                                            llval,
+                                            enum_value_lower_bound,
+                                        );
+                                        bx.assume(cmp_start);
+                                    }
+
+                                    let enum_value_upper_bound =
                                         bx.cx().const_uint_big(ll_t_in, *scalar.valid_range.end());
-                                    let cmp = bx.icmp(IntPredicate::IntULE, llval, ll_t_in_const);
-                                    bx.assume(cmp);
+                                    let cmp_end = bx.icmp(
+                                        IntPredicate::IntULE,
+                                        llval,
+                                        enum_value_upper_bound,
+                                    );
+                                    bx.assume(cmp_end);
                                 }
                             }
                         }
diff --git a/src/test/codegen/enum-bounds-check-derived-idx.rs b/src/test/codegen/enum-bounds-check-derived-idx.rs
new file mode 100644
index 00000000000..7e3773b6a3e
--- /dev/null
+++ b/src/test/codegen/enum-bounds-check-derived-idx.rs
@@ -0,0 +1,25 @@
+// This test checks an optimization that is not guaranteed to work. This test case should not block
+// a future LLVM update.
+// compile-flags: -O
+// min-llvm-version: 11.0
+
+#![crate_type = "lib"]
+
+pub enum Bar {
+    A = 1,
+    B = 3,
+}
+
+// CHECK-LABEL: @lookup_inc
+#[no_mangle]
+pub fn lookup_inc(buf: &[u8; 5], f: Bar) -> u8 {
+    // CHECK-NOT: panic_bounds_check
+    buf[f as usize + 1]
+}
+
+// CHECK-LABEL: @lookup_dec
+#[no_mangle]
+pub fn lookup_dec(buf: &[u8; 5], f: Bar) -> u8 {
+    // CHECK-NOT: panic_bounds_check
+    buf[f as usize - 1]
+}
diff --git a/src/test/codegen/enum-bounds-check-issue-13926.rs b/src/test/codegen/enum-bounds-check-issue-13926.rs
new file mode 100644
index 00000000000..ad029f0fa73
--- /dev/null
+++ b/src/test/codegen/enum-bounds-check-issue-13926.rs
@@ -0,0 +1,19 @@
+// This test checks an optimization that is not guaranteed to work. This test case should not block
+// a future LLVM update.
+// compile-flags: -O
+// min-llvm-version: 11.0
+
+#![crate_type = "lib"]
+
+#[repr(u8)]
+pub enum Exception {
+    Low = 5,
+    High = 10,
+}
+
+// CHECK-LABEL: @access
+#[no_mangle]
+pub fn access(array: &[usize; 12], exc: Exception) -> usize {
+    // CHECK-NOT: panic_bounds_check
+    array[(exc as u8 - 4) as usize]
+}
diff --git a/src/test/codegen/enum-bounds-check.rs b/src/test/codegen/enum-bounds-check.rs
index 21a27c9f35d..17322d5911b 100644
--- a/src/test/codegen/enum-bounds-check.rs
+++ b/src/test/codegen/enum-bounds-check.rs
@@ -12,3 +12,15 @@ pub fn lookup(buf: &[u8; 2], f: Foo) -> u8 {
     // CHECK-NOT: panic_bounds_check
     buf[f as usize]
 }
+
+pub enum Bar {
+    A = 2,
+    B = 3
+}
+
+// CHECK-LABEL: @lookup_unmodified
+#[no_mangle]
+pub fn lookup_unmodified(buf: &[u8; 5], f: Bar) -> u8 {
+    // CHECK-NOT: panic_bounds_check
+    buf[f as usize]
+}