Skip to content

Commit ee4efa1

Browse files
authored
Rollup merge of #141252 - dianqk:gvn-repeat-index, r=saethlin
gvn: bail out unavoidable non-ssa locals in repeat Fixes #141251. We cannot transform `*elem` to `array[idx1]` in the following code, as `idx1` has already been modified. ```rust mir! { let array; let elem; { array = [*val; 5]; elem = &array[idx1]; idx1 = idx2; RET = *elem; Return() } } ``` Perhaps I could transform it to `array[0]`, but I prefer the conservative approach. r? mir-opt
2 parents 0c2fbe5 + be5d6c5 commit ee4efa1

File tree

4 files changed

+91
-1
lines changed

4 files changed

+91
-1
lines changed

compiler/rustc_mir_transform/src/gvn.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -638,6 +638,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
638638
place: PlaceRef<'tcx>,
639639
value: VnIndex,
640640
proj: PlaceElem<'tcx>,
641+
from_non_ssa_index: &mut bool,
641642
) -> Option<VnIndex> {
642643
let proj = match proj {
643644
ProjectionElem::Deref => {
@@ -682,6 +683,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
682683
}
683684
ProjectionElem::Index(idx) => {
684685
if let Value::Repeat(inner, _) = self.get(value) {
686+
*from_non_ssa_index |= self.locals[idx].is_none();
685687
return Some(*inner);
686688
}
687689
let idx = self.locals[idx]?;
@@ -774,6 +776,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
774776

775777
// Invariant: `value` holds the value up-to the `index`th projection excluded.
776778
let mut value = self.locals[place.local]?;
779+
let mut from_non_ssa_index = false;
777780
for (index, proj) in place.projection.iter().enumerate() {
778781
if let Value::Projection(pointer, ProjectionElem::Deref) = *self.get(value)
779782
&& let Value::Address { place: mut pointee, kind, .. } = *self.get(pointer)
@@ -791,7 +794,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
791794
}
792795

793796
let base = PlaceRef { local: place.local, projection: &place.projection[..index] };
794-
value = self.project(base, value, proj)?;
797+
value = self.project(base, value, proj, &mut from_non_ssa_index)?;
795798
}
796799

797800
if let Value::Projection(pointer, ProjectionElem::Deref) = *self.get(value)
@@ -804,6 +807,9 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
804807
}
805808
if let Some(new_local) = self.try_as_local(value, location) {
806809
place_ref = PlaceRef { local: new_local, projection: &[] };
810+
} else if from_non_ssa_index {
811+
// If access to non-SSA locals is unavoidable, bail out.
812+
return None;
807813
}
808814

809815
if place_ref.local != place.local || place_ref.projection.len() < place.projection.len() {
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
- // MIR for `repeat_local` before GVN
2+
+ // MIR for `repeat_local` after GVN
3+
4+
fn repeat_local(_1: usize, _2: usize, _3: i32) -> i32 {
5+
let mut _0: i32;
6+
let mut _4: [i32; 5];
7+
let mut _5: &i32;
8+
9+
bb0: {
10+
_4 = [copy _3; 5];
11+
_5 = &_4[_1];
12+
_1 = copy _2;
13+
- _0 = copy (*_5);
14+
+ _0 = copy _3;
15+
return;
16+
}
17+
}
18+
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
- // MIR for `repeat_place` before GVN
2+
+ // MIR for `repeat_place` after GVN
3+
4+
fn repeat_place(_1: usize, _2: usize, _3: &i32) -> i32 {
5+
let mut _0: i32;
6+
let mut _4: [i32; 5];
7+
let mut _5: &i32;
8+
9+
bb0: {
10+
_4 = [copy (*_3); 5];
11+
_5 = &_4[_1];
12+
_1 = copy _2;
13+
_0 = copy (*_5);
14+
return;
15+
}
16+
}
17+

tests/mir-opt/gvn_repeat.rs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
//@ test-mir-pass: GVN
2+
3+
#![feature(custom_mir, core_intrinsics)]
4+
5+
// Check that we do not introduce out-of-bounds access.
6+
7+
use std::intrinsics::mir::*;
8+
9+
// EMIT_MIR gvn_repeat.repeat_place.GVN.diff
10+
#[custom_mir(dialect = "runtime")]
11+
pub fn repeat_place(mut idx1: usize, idx2: usize, val: &i32) -> i32 {
12+
// CHECK-LABEL: fn repeat_place(
13+
// CHECK: let mut [[ELEM:.*]]: &i32;
14+
// CHECK: _0 = copy (*[[ELEM]])
15+
mir! {
16+
let array;
17+
let elem;
18+
{
19+
array = [*val; 5];
20+
elem = &array[idx1];
21+
idx1 = idx2;
22+
RET = *elem;
23+
Return()
24+
}
25+
}
26+
}
27+
28+
// EMIT_MIR gvn_repeat.repeat_local.GVN.diff
29+
#[custom_mir(dialect = "runtime")]
30+
pub fn repeat_local(mut idx1: usize, idx2: usize, val: i32) -> i32 {
31+
// CHECK-LABEL: fn repeat_local(
32+
// CHECK: _0 = copy _3
33+
mir! {
34+
let array;
35+
let elem;
36+
{
37+
array = [val; 5];
38+
elem = &array[idx1];
39+
idx1 = idx2;
40+
RET = *elem;
41+
Return()
42+
}
43+
}
44+
}
45+
46+
fn main() {
47+
assert_eq!(repeat_place(0, 5, &0), 0);
48+
assert_eq!(repeat_local(0, 5, 0), 0);
49+
}

0 commit comments

Comments
 (0)