Auto merge of #136450 - compiler-errors:simplify-cast, r=saethlin
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:
```rust
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 ();
/* ... */
// Deref a box
_10 = copy ((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>) as *const () (Transmute);
_4 = copy _10;
_6 = copy _4;
// Inlined body of `slice::from_raw_parts`, to turn a unit pointer into a slice-of-unit pointer
_5 = *const [()] from (copy _6, copy _11);
_8 = copy _5;
// Cast the raw slice-of-unit pointer back to a unit pointer
_7 = copy _8 as *mut () (PtrToPtr);
```
A malformed optimization was changing `_7` (which casted the slice-of-unit ptr to a unit ptr) 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
Fixes #135997
This commit is contained in:
bors
commit
550e035a59
4 changed files with 294 additions and 9 deletions
|
|
@ -1367,16 +1367,17 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
|
|||
|
||||
fn simplify_cast(
|
||||
&mut self,
|
||||
kind: &mut CastKind,
|
||||
operand: &mut Operand<'tcx>,
|
||||
initial_kind: &mut CastKind,
|
||||
initial_operand: &mut Operand<'tcx>,
|
||||
to: Ty<'tcx>,
|
||||
location: Location,
|
||||
) -> Option<VnIndex> {
|
||||
use CastKind::*;
|
||||
use rustc_middle::ty::adjustment::PointerCoercion::*;
|
||||
|
||||
let mut from = operand.ty(self.local_decls, self.tcx);
|
||||
let mut value = self.simplify_operand(operand, location)?;
|
||||
let mut from = initial_operand.ty(self.local_decls, self.tcx);
|
||||
let mut kind = *initial_kind;
|
||||
let mut value = self.simplify_operand(initial_operand, location)?;
|
||||
if from == to {
|
||||
return Some(value);
|
||||
}
|
||||
|
|
@ -1400,7 +1401,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
|
|||
&& to.is_unsafe_ptr()
|
||||
&& self.pointers_have_same_metadata(from, to)
|
||||
{
|
||||
*kind = PtrToPtr;
|
||||
kind = PtrToPtr;
|
||||
was_updated_this_iteration = true;
|
||||
}
|
||||
|
||||
|
|
@ -1443,7 +1444,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
|
|||
to: inner_to,
|
||||
} = *self.get(value)
|
||||
{
|
||||
let new_kind = match (inner_kind, *kind) {
|
||||
let new_kind = match (inner_kind, kind) {
|
||||
// Even if there's a narrowing cast in here that's fine, because
|
||||
// things like `*mut [i32] -> *mut i32 -> *const i32` and
|
||||
// `*mut [i32] -> *const [i32] -> *const i32` can skip the middle in MIR.
|
||||
|
|
@ -1471,7 +1472,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
|
|||
_ => None,
|
||||
};
|
||||
if let Some(new_kind) = new_kind {
|
||||
*kind = new_kind;
|
||||
kind = new_kind;
|
||||
from = inner_from;
|
||||
value = inner_value;
|
||||
was_updated_this_iteration = true;
|
||||
|
|
@ -1489,10 +1490,11 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
|
|||
}
|
||||
|
||||
if was_ever_updated && let Some(op) = self.try_as_operand(value, location) {
|
||||
*operand = op;
|
||||
*initial_operand = op;
|
||||
*initial_kind = kind;
|
||||
}
|
||||
|
||||
Some(self.insert(Value::Cast { kind: *kind, value, from, to }))
|
||||
Some(self.insert(Value::Cast { kind, value, from, to }))
|
||||
}
|
||||
|
||||
fn simplify_len(&mut self, place: &mut Place<'tcx>, location: Location) -> Option<VnIndex> {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,15 @@
|
|||
// skip-filecheck
|
||||
//@ compile-flags: -Zmir-enable-passes=+Inline,+GVN --crate-type lib
|
||||
// EMIT_MIR_FOR_EACH_PANIC_STRATEGY
|
||||
// EMIT_MIR dont_reset_cast_kind_without_updating_operand.test.GVN.diff
|
||||
|
||||
fn test() {
|
||||
let vp_ctx: &Box<()> = &Box::new(());
|
||||
let slf: *const () = &raw const **vp_ctx;
|
||||
let bytes = std::ptr::slice_from_raw_parts(slf, 1);
|
||||
let _x = foo(bytes);
|
||||
}
|
||||
|
||||
fn foo(bytes: *const [()]) -> *mut () {
|
||||
bytes as *mut ()
|
||||
}
|
||||
|
|
@ -0,0 +1,180 @@
|
|||
- // MIR for `test` before GVN
|
||||
+ // MIR for `test` after GVN
|
||||
|
||||
fn test() -> () {
|
||||
let mut _0: ();
|
||||
let _1: &std::boxed::Box<()>;
|
||||
let _2: &std::boxed::Box<()>;
|
||||
let _3: std::boxed::Box<()>;
|
||||
let mut _6: *const ();
|
||||
let mut _8: *const [()];
|
||||
let mut _9: std::boxed::Box<()>;
|
||||
let mut _10: *const ();
|
||||
let mut _23: usize;
|
||||
scope 1 {
|
||||
debug vp_ctx => _1;
|
||||
let _4: *const ();
|
||||
scope 2 {
|
||||
debug slf => _10;
|
||||
let _5: *const [()];
|
||||
scope 3 {
|
||||
debug bytes => _5;
|
||||
let _7: *mut ();
|
||||
scope 4 {
|
||||
debug _x => _7;
|
||||
}
|
||||
scope 18 (inlined foo) {
|
||||
}
|
||||
}
|
||||
scope 16 (inlined slice_from_raw_parts::<()>) {
|
||||
scope 17 (inlined std::ptr::from_raw_parts::<[()], ()>) {
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
scope 5 (inlined Box::<()>::new) {
|
||||
let mut _11: usize;
|
||||
let mut _12: usize;
|
||||
let mut _13: *mut u8;
|
||||
scope 6 (inlined alloc::alloc::exchange_malloc) {
|
||||
let _14: std::alloc::Layout;
|
||||
let mut _15: std::result::Result<std::ptr::NonNull<[u8]>, std::alloc::AllocError>;
|
||||
let mut _16: isize;
|
||||
let mut _18: !;
|
||||
scope 7 {
|
||||
let _17: std::ptr::NonNull<[u8]>;
|
||||
scope 8 {
|
||||
scope 11 (inlined NonNull::<[u8]>::as_mut_ptr) {
|
||||
scope 12 (inlined NonNull::<[u8]>::as_non_null_ptr) {
|
||||
scope 13 (inlined NonNull::<[u8]>::cast::<u8>) {
|
||||
let mut _22: *mut [u8];
|
||||
scope 14 (inlined NonNull::<[u8]>::as_ptr) {
|
||||
}
|
||||
}
|
||||
}
|
||||
scope 15 (inlined NonNull::<u8>::as_ptr) {
|
||||
}
|
||||
}
|
||||
}
|
||||
scope 10 (inlined <std::alloc::Global as Allocator>::allocate) {
|
||||
}
|
||||
}
|
||||
scope 9 (inlined Layout::from_size_align_unchecked) {
|
||||
let mut _19: bool;
|
||||
let _20: ();
|
||||
let mut _21: std::ptr::Alignment;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
bb0: {
|
||||
StorageLive(_1);
|
||||
- StorageLive(_2);
|
||||
+ nop;
|
||||
StorageLive(_3);
|
||||
StorageLive(_11);
|
||||
StorageLive(_12);
|
||||
StorageLive(_13);
|
||||
- _11 = SizeOf(());
|
||||
- _12 = AlignOf(());
|
||||
+ _11 = const 0_usize;
|
||||
+ _12 = const 1_usize;
|
||||
StorageLive(_14);
|
||||
StorageLive(_16);
|
||||
StorageLive(_17);
|
||||
StorageLive(_19);
|
||||
_19 = const false;
|
||||
- switchInt(move _19) -> [0: bb6, otherwise: bb5];
|
||||
+ switchInt(const false) -> [0: bb6, otherwise: bb5];
|
||||
}
|
||||
|
||||
bb1: {
|
||||
StorageDead(_3);
|
||||
StorageDead(_1);
|
||||
return;
|
||||
}
|
||||
|
||||
bb2: {
|
||||
unreachable;
|
||||
}
|
||||
|
||||
bb3: {
|
||||
- _18 = handle_alloc_error(move _14) -> unwind unreachable;
|
||||
+ _18 = handle_alloc_error(const Layout {{ size: 0_usize, align: std::ptr::Alignment(std::ptr::alignment::AlignmentEnum::_Align1Shl0) }}) -> unwind unreachable;
|
||||
}
|
||||
|
||||
bb4: {
|
||||
_17 = copy ((_15 as Ok).0: std::ptr::NonNull<[u8]>);
|
||||
StorageLive(_22);
|
||||
_22 = copy _17 as *mut [u8] (Transmute);
|
||||
_13 = copy _22 as *mut u8 (PtrToPtr);
|
||||
StorageDead(_22);
|
||||
StorageDead(_15);
|
||||
StorageDead(_17);
|
||||
StorageDead(_16);
|
||||
StorageDead(_14);
|
||||
_3 = ShallowInitBox(move _13, ());
|
||||
StorageDead(_13);
|
||||
StorageDead(_12);
|
||||
StorageDead(_11);
|
||||
_2 = &_3;
|
||||
_1 = copy _2;
|
||||
- StorageDead(_2);
|
||||
+ nop;
|
||||
StorageLive(_4);
|
||||
- _9 = deref_copy _3;
|
||||
+ _9 = copy _3;
|
||||
_10 = copy ((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>) as *const () (Transmute);
|
||||
_4 = copy _10;
|
||||
- StorageLive(_5);
|
||||
+ nop;
|
||||
StorageLive(_6);
|
||||
- _6 = copy _4;
|
||||
+ _6 = copy _10;
|
||||
StorageLive(_23);
|
||||
_23 = const 1_usize;
|
||||
- _5 = *const [()] from (copy _6, copy _23);
|
||||
+ _5 = *const [()] from (copy _10, const 1_usize);
|
||||
StorageDead(_23);
|
||||
StorageDead(_6);
|
||||
StorageLive(_7);
|
||||
StorageLive(_8);
|
||||
_8 = copy _5;
|
||||
- _7 = copy _8 as *mut () (PtrToPtr);
|
||||
+ _7 = copy _5 as *mut () (PtrToPtr);
|
||||
StorageDead(_8);
|
||||
StorageDead(_7);
|
||||
- StorageDead(_5);
|
||||
+ nop;
|
||||
StorageDead(_4);
|
||||
drop(_3) -> [return: bb1, unwind unreachable];
|
||||
}
|
||||
|
||||
bb5: {
|
||||
- _20 = Layout::from_size_align_unchecked::precondition_check(copy _11, copy _12) -> [return: bb6, unwind unreachable];
|
||||
+ _20 = Layout::from_size_align_unchecked::precondition_check(const 0_usize, const 1_usize) -> [return: bb6, unwind unreachable];
|
||||
}
|
||||
|
||||
bb6: {
|
||||
StorageDead(_19);
|
||||
StorageLive(_21);
|
||||
- _21 = copy _12 as std::ptr::Alignment (Transmute);
|
||||
- _14 = Layout { size: copy _11, align: move _21 };
|
||||
+ _21 = const std::ptr::Alignment(std::ptr::alignment::AlignmentEnum::_Align1Shl0);
|
||||
+ _14 = const Layout {{ size: 0_usize, align: std::ptr::Alignment(std::ptr::alignment::AlignmentEnum::_Align1Shl0) }};
|
||||
StorageDead(_21);
|
||||
StorageLive(_15);
|
||||
- _15 = std::alloc::Global::alloc_impl(const alloc::alloc::exchange_malloc::promoted[0], copy _14, const false) -> [return: bb7, unwind unreachable];
|
||||
+ _15 = std::alloc::Global::alloc_impl(const alloc::alloc::exchange_malloc::promoted[0], const Layout {{ size: 0_usize, align: std::ptr::Alignment(std::ptr::alignment::AlignmentEnum::_Align1Shl0) }}, const false) -> [return: bb7, unwind unreachable];
|
||||
}
|
||||
|
||||
bb7: {
|
||||
_16 = discriminant(_15);
|
||||
switchInt(move _16) -> [0: bb4, 1: bb3, otherwise: bb2];
|
||||
}
|
||||
+ }
|
||||
+
|
||||
+ ALLOC0 (size: 8, align: 4) {
|
||||
+ 01 00 00 00 00 00 00 00 │ ........
|
||||
}
|
||||
|
||||
|
|
@ -0,0 +1,88 @@
|
|||
- // MIR for `test` before GVN
|
||||
+ // MIR for `test` after GVN
|
||||
|
||||
fn test() -> () {
|
||||
let mut _0: ();
|
||||
let _1: &std::boxed::Box<()>;
|
||||
let _2: &std::boxed::Box<()>;
|
||||
let _3: std::boxed::Box<()>;
|
||||
let mut _6: *const ();
|
||||
let mut _8: *const [()];
|
||||
let mut _9: std::boxed::Box<()>;
|
||||
let mut _10: *const ();
|
||||
let mut _11: usize;
|
||||
scope 1 {
|
||||
debug vp_ctx => _1;
|
||||
let _4: *const ();
|
||||
scope 2 {
|
||||
debug slf => _10;
|
||||
let _5: *const [()];
|
||||
scope 3 {
|
||||
debug bytes => _5;
|
||||
let _7: *mut ();
|
||||
scope 4 {
|
||||
debug _x => _7;
|
||||
}
|
||||
scope 7 (inlined foo) {
|
||||
}
|
||||
}
|
||||
scope 5 (inlined slice_from_raw_parts::<()>) {
|
||||
scope 6 (inlined std::ptr::from_raw_parts::<[()], ()>) {
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
bb0: {
|
||||
StorageLive(_1);
|
||||
- StorageLive(_2);
|
||||
+ nop;
|
||||
StorageLive(_3);
|
||||
_3 = Box::<()>::new(const ()) -> [return: bb1, unwind continue];
|
||||
}
|
||||
|
||||
bb1: {
|
||||
_2 = &_3;
|
||||
_1 = copy _2;
|
||||
- StorageDead(_2);
|
||||
+ nop;
|
||||
StorageLive(_4);
|
||||
- _9 = deref_copy _3;
|
||||
+ _9 = copy _3;
|
||||
_10 = copy ((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>) as *const () (Transmute);
|
||||
_4 = copy _10;
|
||||
- StorageLive(_5);
|
||||
+ nop;
|
||||
StorageLive(_6);
|
||||
- _6 = copy _4;
|
||||
+ _6 = copy _10;
|
||||
StorageLive(_11);
|
||||
_11 = const 1_usize;
|
||||
- _5 = *const [()] from (copy _6, copy _11);
|
||||
+ _5 = *const [()] from (copy _10, const 1_usize);
|
||||
StorageDead(_11);
|
||||
StorageDead(_6);
|
||||
StorageLive(_7);
|
||||
StorageLive(_8);
|
||||
_8 = copy _5;
|
||||
- _7 = copy _8 as *mut () (PtrToPtr);
|
||||
+ _7 = copy _5 as *mut () (PtrToPtr);
|
||||
StorageDead(_8);
|
||||
StorageDead(_7);
|
||||
- StorageDead(_5);
|
||||
+ nop;
|
||||
StorageDead(_4);
|
||||
drop(_3) -> [return: bb2, unwind: bb3];
|
||||
}
|
||||
|
||||
bb2: {
|
||||
StorageDead(_3);
|
||||
StorageDead(_1);
|
||||
return;
|
||||
}
|
||||
|
||||
bb3 (cleanup): {
|
||||
resume;
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
Add table
Add a link
Reference in a new issue