about summary refs log tree commit diff
path: root/compiler
diff options
context:
space:
mode:
authorYuki Okushi <huyuumi.dev@gmail.com>2020-12-17 11:43:59 +0900
committerGitHub <noreply@github.com>2020-12-17 11:43:59 +0900
commita611f8dbfc89734722d160c70983b9595f8872ae (patch)
tree35dadda7231a13f32f09728949925de87952b2e7 /compiler
parent3d42c00f0bbe76ea47081809b5525bda4f2149dd (diff)
parent3812f70355132b53092e826c9d1a753dfd8a1874 (diff)
downloadrust-a611f8dbfc89734722d160c70983b9595f8872ae.tar.gz
rust-a611f8dbfc89734722d160c70983b9595f8872ae.zip
Rollup merge of #79882 - wecing:master, r=oli-obk
Fix issue #78496

EarlyOtherwiseBranch finds MIR structures like:

```
bb0: {
  ...
  _2 = discriminant(X)
  ...
  switchInt(_2) -> [1_isize: bb1, otherwise: bb3]
}
bb1: {
  ...
  _3 = discriminant(Y)
  ...
  switchInt(_3) -> [1_isize: bb2, otherwise: bb3]
}
bb2: {...}
bb3: {...}
```

And transforms them into something like:

```
bb0: {
  ...
  _2 = discriminant(X)
  _3 = discriminant(Y)
  _4 = Eq(_2, _3)
  switchInt(_4) -> [true: bb4, otherwise: bb3]
}
bb2: {...} // unchanged
bb3: {...} // unchanged
bb4: {
  switchInt(_2) -> [1_isize: bb2, otherwise: bb3]
}
```

But that is not always a safe thing to do -- sometimes the early `otherwise` branch is necessary so the later block could assume the value of `discriminant(X)`.

I am not totally sure what's the best way to detect that, but fixing #78496 should be easy -- we just check if `X` is a sub-expression of `Y`. A more precise test might be to check if `Y` contains a `Downcast(1)` of `X`, but I think this might be good enough.

Fix #78496
Diffstat (limited to 'compiler')
-rw-r--r--compiler/rustc_mir/src/transform/early_otherwise_branch.rs27
1 files changed, 27 insertions, 0 deletions
diff --git a/compiler/rustc_mir/src/transform/early_otherwise_branch.rs b/compiler/rustc_mir/src/transform/early_otherwise_branch.rs
index 6fbcc140978..b16a99d7f0d 100644
--- a/compiler/rustc_mir/src/transform/early_otherwise_branch.rs
+++ b/compiler/rustc_mir/src/transform/early_otherwise_branch.rs
@@ -284,6 +284,33 @@ impl<'a, 'tcx> Helper<'a, 'tcx> {
                 return None;
             }
 
+            // when the second place is a projection of the first one, it's not safe to calculate their discriminant values sequentially.
+            // for example, this should not be optimized:
+            //
+            // ```rust
+            // enum E<'a> { Empty, Some(&'a E<'a>), }
+            // let Some(Some(_)) = e;
+            // ```
+            //
+            // ```mir
+            // bb0: {
+            //   _2 = discriminant(*_1)
+            //   switchInt(_2) -> [...]
+            // }
+            // bb1: {
+            //   _3 = discriminant(*(((*_1) as Some).0: &E))
+            //   switchInt(_3) -> [...]
+            // }
+            // ```
+            let discr_place = discr_info.place_of_adt_discr_read;
+            let this_discr_place = this_bb_discr_info.place_of_adt_discr_read;
+            if discr_place.local == this_discr_place.local
+                && this_discr_place.projection.starts_with(discr_place.projection)
+            {
+                trace!("NO: one target is the projection of another");
+                return None;
+            }
+
             // if we reach this point, the optimization applies, and we should be able to optimize this case
             // store the info that is needed to apply the optimization