Skip to content

Commit eed322b

Browse files
committed
Auto merge of #136450 - compiler-errors:simplify-cast, r=<try>
Don't reset cast kind without also updating the operand in `simplify_cast` in GVN Consider this heavily elided segment of the pre-GVN example code that was committed as a test: ``` let _4: *const (); let _5: *const [()]; let mut _6: *const (); let _7: *mut (); let mut _8: *const [()]; let mut _9: std::boxed::Box<()>; let mut _10: *const (); /* ... */ _10 = copy ((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>) as *const () (Transmute); _4 = copy _10; _6 = copy _4; _5 = *const [()] from (copy _6, copy _11); _8 = copy _5; _7 = copy _8 as *mut () (PtrToPtr); ``` A malformed optimization was changing `_7` to: ``` _7 = copy _5 as *mut () (Transmute); ``` (where `_8` was just replaced with `_5` bc of simple copy propagation, that part is not important... the CastKind changing to Transmute is the important part here). In #133324, two new functionalities were implemented: * Peeking through unsized -> sized PtrToPtr casts whose operand is `AggregateKind::RawPtr`, to turn it into PtrToPtr casts of the base of the aggregate. In this case, this allows us to see that the value of `_7` is just a ptr-to-ptr cast of `_6`. * Folding a PtrToPtr cast of an operand which is a Transmute cast into just a single Transmute, which (theoretically) allows us to treat `_7` as a transmute into `*mut ()` of the base of the cast of `_10`, which is the place projection of `((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>)`. However, when applying those two subsequent optimizations, we must *not* update the CastKind of the final cast *unless* we also update the operand of the cast, since the operand may no longer make sense with the updated CastKind. In this case, this is problematic because the type of `_8` is `*const [()]`, but that operand in assignment statement of `_7` does *not* get turned into something like `((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>)` -- **in other words, `try_to_operand` fails** -- because GVN only turns value nodes into locals or consts, not projections of locals. So we fail to update the operand, but we still update the CastKind to Transmute, which means we now are transmuting types of different sizes (a wide pointer and a thin pointer). r? `@scottmcm` or `@cjgillot` Fixes #136361
2 parents 6c0de7f + a607ae7 commit eed322b

File tree

3 files changed

+113
-9
lines changed

3 files changed

+113
-9
lines changed

compiler/rustc_mir_transform/src/gvn.rs

+11-9
Original file line numberDiff line numberDiff line change
@@ -1366,16 +1366,17 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
13661366

13671367
fn simplify_cast(
13681368
&mut self,
1369-
kind: &mut CastKind,
1370-
operand: &mut Operand<'tcx>,
1369+
initial_kind: &mut CastKind,
1370+
initial_operand: &mut Operand<'tcx>,
13711371
to: Ty<'tcx>,
13721372
location: Location,
13731373
) -> Option<VnIndex> {
13741374
use CastKind::*;
13751375
use rustc_middle::ty::adjustment::PointerCoercion::*;
13761376

1377-
let mut from = operand.ty(self.local_decls, self.tcx);
1378-
let mut value = self.simplify_operand(operand, location)?;
1377+
let mut from = initial_operand.ty(self.local_decls, self.tcx);
1378+
let mut kind = *initial_kind;
1379+
let mut value = self.simplify_operand(initial_operand, location)?;
13791380
if from == to {
13801381
return Some(value);
13811382
}
@@ -1399,7 +1400,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
13991400
&& to.is_unsafe_ptr()
14001401
&& self.pointers_have_same_metadata(from, to)
14011402
{
1402-
*kind = PtrToPtr;
1403+
kind = PtrToPtr;
14031404
was_updated_this_iteration = true;
14041405
}
14051406

@@ -1442,7 +1443,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
14421443
to: inner_to,
14431444
} = *self.get(value)
14441445
{
1445-
let new_kind = match (inner_kind, *kind) {
1446+
let new_kind = match (inner_kind, kind) {
14461447
// Even if there's a narrowing cast in here that's fine, because
14471448
// things like `*mut [i32] -> *mut i32 -> *const i32` and
14481449
// `*mut [i32] -> *const [i32] -> *const i32` can skip the middle in MIR.
@@ -1470,7 +1471,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
14701471
_ => None,
14711472
};
14721473
if let Some(new_kind) = new_kind {
1473-
*kind = new_kind;
1474+
kind = new_kind;
14741475
from = inner_from;
14751476
value = inner_value;
14761477
was_updated_this_iteration = true;
@@ -1488,10 +1489,11 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
14881489
}
14891490

14901491
if was_ever_updated && let Some(op) = self.try_as_operand(value, location) {
1491-
*operand = op;
1492+
*initial_operand = op;
1493+
*initial_kind = kind;
14921494
}
14931495

1494-
Some(self.insert(Value::Cast { kind: *kind, value, from, to }))
1496+
Some(self.insert(Value::Cast { kind, value, from, to }))
14951497
}
14961498

14971499
fn simplify_len(&mut self, place: &mut Place<'tcx>, location: Location) -> Option<VnIndex> {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// skip-filecheck
2+
//@ compile-flags: -Zmir-enable-passes=+Inline,+GVN --crate-type lib
3+
// EMIT_MIR dont_reset_cast_kind_without_updating_operand.test.GVN.diff
4+
5+
fn test() {
6+
let vp_ctx: &Box<()> = &Box::new(());
7+
let slf: *const () = &raw const **vp_ctx;
8+
let bytes = std::ptr::slice_from_raw_parts(slf, 1);
9+
let _x = foo(bytes);
10+
}
11+
12+
fn foo(bytes: *const [()]) -> *mut () {
13+
bytes as *mut ()
14+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
- // MIR for `test` before GVN
2+
+ // MIR for `test` after GVN
3+
4+
fn test() -> () {
5+
let mut _0: ();
6+
let _1: &std::boxed::Box<()>;
7+
let _2: &std::boxed::Box<()>;
8+
let _3: std::boxed::Box<()>;
9+
let mut _6: *const ();
10+
let mut _8: *const [()];
11+
let mut _9: std::boxed::Box<()>;
12+
let mut _10: *const ();
13+
let mut _11: usize;
14+
scope 1 {
15+
debug vp_ctx => _1;
16+
let _4: *const ();
17+
scope 2 {
18+
debug slf => _10;
19+
let _5: *const [()];
20+
scope 3 {
21+
debug bytes => _5;
22+
let _7: *mut ();
23+
scope 4 {
24+
debug _x => _7;
25+
}
26+
scope 7 (inlined foo) {
27+
}
28+
}
29+
scope 5 (inlined slice_from_raw_parts::<()>) {
30+
scope 6 (inlined std::ptr::from_raw_parts::<[()], ()>) {
31+
}
32+
}
33+
}
34+
}
35+
36+
bb0: {
37+
StorageLive(_1);
38+
- StorageLive(_2);
39+
+ nop;
40+
StorageLive(_3);
41+
_3 = Box::<()>::new(const ()) -> [return: bb1, unwind continue];
42+
}
43+
44+
bb1: {
45+
_2 = &_3;
46+
_1 = copy _2;
47+
- StorageDead(_2);
48+
+ nop;
49+
StorageLive(_4);
50+
- _9 = deref_copy _3;
51+
+ _9 = copy _3;
52+
_10 = copy ((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>) as *const () (Transmute);
53+
_4 = copy _10;
54+
- StorageLive(_5);
55+
+ nop;
56+
StorageLive(_6);
57+
- _6 = copy _4;
58+
+ _6 = copy _10;
59+
StorageLive(_11);
60+
_11 = const 1_usize;
61+
- _5 = *const [()] from (copy _6, copy _11);
62+
+ _5 = *const [()] from (copy _10, const 1_usize);
63+
StorageDead(_11);
64+
StorageDead(_6);
65+
StorageLive(_7);
66+
StorageLive(_8);
67+
_8 = copy _5;
68+
- _7 = copy _8 as *mut () (PtrToPtr);
69+
+ _7 = copy _5 as *mut () (PtrToPtr);
70+
StorageDead(_8);
71+
StorageDead(_7);
72+
- StorageDead(_5);
73+
+ nop;
74+
StorageDead(_4);
75+
drop(_3) -> [return: bb2, unwind: bb3];
76+
}
77+
78+
bb2: {
79+
StorageDead(_3);
80+
StorageDead(_1);
81+
return;
82+
}
83+
84+
bb3 (cleanup): {
85+
resume;
86+
}
87+
}
88+

0 commit comments

Comments
 (0)