Skip to content

Commit

Permalink
Auto merge of #126308 - scottmcm:ban-some-coercions, r=saethlin
Browse files Browse the repository at this point in the history
Ban `ArrayToPointer` and `MutToConstPointer` from runtime MIR

Zulip conversation: <https://rust-lang.zulipchat.com/#narrow/stream/189540-t-compiler.2Fwg-mir-opt/topic/CastKind.3A.3APointerCoercion.20in.20Runtime.20MIR/near/443955195>

Apparently MIR borrowck cares about at least one of these for checking variance.

In runtime MIR, though, there's no need for them as `PtrToPtr` does the same thing.

(Banning them simplifies passes like GVN that no longer need to handle multiple cast possibilities.)

r? mir
  • Loading branch information
bors committed Jun 19, 2024
2 parents d8a38b0 + e04e351 commit 3d5d7a2
Show file tree
Hide file tree
Showing 24 changed files with 78 additions and 50 deletions.
21 changes: 11 additions & 10 deletions compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -677,21 +677,22 @@ fn codegen_stmt<'tcx>(
CastKind::PointerCoercion(PointerCoercion::UnsafeFnPointer),
ref operand,
to_ty,
)
| Rvalue::Cast(
CastKind::PointerCoercion(PointerCoercion::MutToConstPointer),
ref operand,
to_ty,
)
| Rvalue::Cast(
CastKind::PointerCoercion(PointerCoercion::ArrayToPointer),
ref operand,
to_ty,
) => {
let to_layout = fx.layout_of(fx.monomorphize(to_ty));
let operand = codegen_operand(fx, operand);
lval.write_cvalue(fx, operand.cast_pointer_to(to_layout));
}
Rvalue::Cast(
CastKind::PointerCoercion(
PointerCoercion::MutToConstPointer | PointerCoercion::ArrayToPointer,
),
..,
) => {
bug!(
"{:?} is for borrowck, and should never appear in codegen",
to_place_and_rval.1
);
}
Rvalue::Cast(
CastKind::IntToInt
| CastKind::FloatToFloat
Expand Down
11 changes: 6 additions & 5 deletions compiler/rustc_codegen_ssa/src/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -456,8 +456,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
base::unsize_ptr(bx, lldata, operand.layout.ty, cast.ty, llextra);
OperandValue::Pair(lldata, llextra)
}
mir::CastKind::PointerCoercion(PointerCoercion::MutToConstPointer)
| mir::CastKind::PtrToPtr
mir::CastKind::PointerCoercion(
PointerCoercion::MutToConstPointer | PointerCoercion::ArrayToPointer,
) => {
bug!("{kind:?} is for borrowck, and should never appear in codegen");
}
mir::CastKind::PtrToPtr
if bx.cx().is_backend_scalar_pair(operand.layout) =>
{
if let OperandValue::Pair(data_ptr, meta) = operand.val {
Expand All @@ -477,9 +481,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
base::cast_to_dyn_star(bx, lldata, operand.layout, cast.ty, llextra);
OperandValue::Pair(lldata, llextra)
}
mir::CastKind::PointerCoercion(
PointerCoercion::MutToConstPointer | PointerCoercion::ArrayToPointer,
)
| mir::CastKind::IntToInt
| mir::CastKind::FloatToInt
| mir::CastKind::FloatToFloat
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_const_eval/src/interpret/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
CastKind::PointerCoercion(
PointerCoercion::MutToConstPointer | PointerCoercion::ArrayToPointer,
) => {
// These are NOPs, but can be wide pointers.
let v = self.read_immediate(src)?;
self.write_immediate(*v, dest)?;
bug!("{cast_kind:?} casts are for borrowck only, not runtime MIR");
}

CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer) => {
Expand Down
13 changes: 11 additions & 2 deletions compiler/rustc_middle/src/mir/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ pub enum AnalysisPhase {
/// * [`StatementKind::AscribeUserType`]
/// * [`StatementKind::Coverage`] with [`CoverageKind::BlockMarker`] or [`CoverageKind::SpanMarker`]
/// * [`Rvalue::Ref`] with `BorrowKind::Fake`
/// * [`CastKind::PointerCoercion`] with any of the following:
/// * [`PointerCoercion::ArrayToPointer`]
/// * [`PointerCoercion::MutToConstPointer`]
///
/// Furthermore, `Deref` projections must be the first projection within any place (if they
/// appear at all)
Expand Down Expand Up @@ -1284,8 +1287,7 @@ pub enum Rvalue<'tcx> {
///
/// This allows for casts from/to a variety of types.
///
/// **FIXME**: Document exactly which `CastKind`s allow which types of casts. Figure out why
/// `ArrayToPointer` and `MutToConstPointer` are special.
/// **FIXME**: Document exactly which `CastKind`s allow which types of casts.
Cast(CastKind, Operand<'tcx>, Ty<'tcx>),

/// * `Offset` has the same semantics as [`offset`](pointer::offset), except that the second
Expand Down Expand Up @@ -1365,6 +1367,13 @@ pub enum CastKind {
PointerWithExposedProvenance,
/// Pointer related casts that are done by coercions. Note that reference-to-raw-ptr casts are
/// translated into `&raw mut/const *r`, i.e., they are not actually casts.
///
/// The following are allowed in [`AnalysisPhase::Initial`] as they're needed for borrowck,
/// but after that are forbidden (including in all phases of runtime MIR):
/// * [`PointerCoercion::ArrayToPointer`]
/// * [`PointerCoercion::MutToConstPointer`]
///
/// Both are runtime nops, so should be [`CastKind::PtrToPtr`] instead in runtime MIR.
PointerCoercion(PointerCoercion),
/// Cast into a dyn* object.
DynStar,
Expand Down
19 changes: 18 additions & 1 deletion compiler/rustc_mir_transform/src/cleanup_post_borrowck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
use crate::MirPass;
use rustc_middle::mir::coverage::CoverageKind;
use rustc_middle::mir::{Body, BorrowKind, Rvalue, StatementKind, TerminatorKind};
use rustc_middle::mir::{Body, BorrowKind, CastKind, Rvalue, StatementKind, TerminatorKind};
use rustc_middle::ty::adjustment::PointerCoercion;
use rustc_middle::ty::TyCtxt;

pub struct CleanupPostBorrowck;
Expand All @@ -36,6 +37,22 @@ impl<'tcx> MirPass<'tcx> for CleanupPostBorrowck {
CoverageKind::BlockMarker { .. } | CoverageKind::SpanMarker { .. },
)
| StatementKind::FakeRead(..) => statement.make_nop(),
StatementKind::Assign(box (
_,
Rvalue::Cast(
ref mut cast_kind @ CastKind::PointerCoercion(
PointerCoercion::ArrayToPointer
| PointerCoercion::MutToConstPointer,
),
..,
),
)) => {
// BorrowCk needed to track whether these cases were coercions or casts,
// to know whether to check lifetimes in their pointees,
// but from now on that distinction doesn't matter,
// so just make them ordinary pointer casts instead.
*cast_kind = CastKind::PtrToPtr;
}
_ => (),
}
}
Expand Down
10 changes: 3 additions & 7 deletions compiler/rustc_mir_transform/src/gvn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -571,11 +571,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
let ret = self.ecx.ptr_to_ptr(&src, to).ok()?;
ret.into()
}
CastKind::PointerCoercion(
ty::adjustment::PointerCoercion::MutToConstPointer
| ty::adjustment::PointerCoercion::ArrayToPointer
| ty::adjustment::PointerCoercion::UnsafeFnPointer,
) => {
CastKind::PointerCoercion(ty::adjustment::PointerCoercion::UnsafeFnPointer) => {
let src = self.evaluated[value].as_ref()?;
let src = self.ecx.read_immediate(src).ok()?;
let to = self.ecx.layout_of(to).ok()?;
Expand Down Expand Up @@ -1164,10 +1160,10 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
}
}

if let PtrToPtr | PointerCoercion(MutToConstPointer) = kind
if let PtrToPtr = kind
&& let Value::Cast { kind: inner_kind, value: inner_value, from: inner_from, to: _ } =
*self.get(value)
&& let PtrToPtr | PointerCoercion(MutToConstPointer) = inner_kind
&& let PtrToPtr = inner_kind
{
from = inner_from;
value = inner_value;
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,11 @@ mod abort_unwinding_calls;
mod add_call_guards;
mod add_moves_for_packed_drops;
mod add_retag;
mod add_subtyping_projections;
mod check_alignment;
mod check_const_item_mutation;
mod check_packed_ref;
mod remove_place_mention;
// This pass is public to allow external drivers to perform MIR cleanup
mod add_subtyping_projections;
pub mod cleanup_post_borrowck;
mod copy_prop;
mod coroutine;
Expand Down Expand Up @@ -94,6 +94,7 @@ mod prettify;
mod promote_consts;
mod ref_prop;
mod remove_noop_landing_pads;
mod remove_place_mention;
mod remove_storage_markers;
mod remove_uninit_drops;
mod remove_unneeded_drops;
Expand All @@ -103,7 +104,6 @@ mod reveal_all;
mod shim;
mod ssa;
// This pass is public to allow external drivers to perform MIR cleanup
mod check_alignment;
pub mod simplify;
mod simplify_branches;
mod simplify_comparison_integral;
Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_mir_transform/src/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1188,6 +1188,9 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
"CastKind::{kind:?} output must be a raw const pointer, not {:?}",
ty::RawPtr(_, Mutability::Not)
);
if self.mir_phase >= MirPhase::Analysis(AnalysisPhase::PostCleanup) {
self.fail(location, format!("After borrowck, MIR disallows {kind:?}"));
}
}
CastKind::PointerCoercion(PointerCoercion::ArrayToPointer) => {
// FIXME: Check pointee types
Expand All @@ -1201,6 +1204,9 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
"CastKind::{kind:?} output must be a raw pointer, not {:?}",
ty::RawPtr(..)
);
if self.mir_phase >= MirPhase::Analysis(AnalysisPhase::PostCleanup) {
self.fail(location, format!("After borrowck, MIR disallows {kind:?}"));
}
}
CastKind::PointerCoercion(PointerCoercion::Unsize) => {
// This is used for all `CoerceUnsized` types,
Expand All @@ -1212,7 +1218,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
if !input_valid || !target_valid {
self.fail(
location,
format!("Wrong cast kind {kind:?} for the type {op_ty}",),
format!("Wrong cast kind {kind:?} for the type {op_ty}"),
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@

bb4: {
StorageDead(_8);
- _11 = _6 as *const [bool; 0] (PointerCoercion(MutToConstPointer));
- _11 = _6 as *const [bool; 0] (PtrToPtr);
- _5 = NonNull::<[bool; 0]> { pointer: _11 };
+ _11 = const {0x1 as *const [bool; 0]};
+ _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@

bb5: {
StorageDead(_8);
- _11 = _6 as *const [bool; 0] (PointerCoercion(MutToConstPointer));
- _11 = _6 as *const [bool; 0] (PtrToPtr);
- _5 = NonNull::<[bool; 0]> { pointer: _11 };
+ _11 = const {0x1 as *const [bool; 0]};
+ _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@

bb4: {
StorageDead(_8);
- _11 = _6 as *const [bool; 0] (PointerCoercion(MutToConstPointer));
- _11 = _6 as *const [bool; 0] (PtrToPtr);
- _5 = NonNull::<[bool; 0]> { pointer: _11 };
+ _11 = const {0x1 as *const [bool; 0]};
+ _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@

bb5: {
StorageDead(_8);
- _11 = _6 as *const [bool; 0] (PointerCoercion(MutToConstPointer));
- _11 = _6 as *const [bool; 0] (PtrToPtr);
- _5 = NonNull::<[bool; 0]> { pointer: _11 };
+ _11 = const {0x1 as *const [bool; 0]};
+ _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
StorageLive(_4);
_4 = _1;
_3 = move _4 as *mut u8 (PtrToPtr);
_2 = move _3 as *const u8 (PointerCoercion(MutToConstPointer));
_2 = move _3 as *const u8 (PtrToPtr);
StorageDead(_4);
StorageDead(_3);
- _0 = move _2 as *const u8 (PtrToPtr);
Expand Down
2 changes: 1 addition & 1 deletion tests/mir-opt/instsimplify/casts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub fn roundtrip(x: *const u8) -> *const u8 {
// CHECK-LABEL: fn roundtrip(
// CHECK: _4 = _1;
// CHECK: _3 = move _4 as *mut u8 (PtrToPtr);
// CHECK: _2 = move _3 as *const u8 (PointerCoercion(MutToConstPointer));
// CHECK: _2 = move _3 as *const u8 (PtrToPtr);
x as *mut u8 as *const u8
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fn slice_get_unchecked_mut_range(_1: &mut [u32], _2: std::ops::Range<usize>) ->
StorageLive(_9);
StorageLive(_7);
StorageLive(_6);
_6 = _5 as *const [u32] (PointerCoercion(MutToConstPointer));
_6 = _5 as *const [u32] (PtrToPtr);
_7 = PtrMetadata(_6);
StorageDead(_6);
_8 = <std::ops::Range<usize> as SliceIndex<[T]>>::get_unchecked_mut::precondition_check(_3, _4, move _7) -> [return: bb1, unwind unreachable];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fn slice_get_unchecked_mut_range(_1: &mut [u32], _2: std::ops::Range<usize>) ->
StorageLive(_9);
StorageLive(_7);
StorageLive(_6);
_6 = _5 as *const [u32] (PointerCoercion(MutToConstPointer));
_6 = _5 as *const [u32] (PtrToPtr);
_7 = PtrMetadata(_6);
StorageDead(_6);
_8 = <std::ops::Range<usize> as SliceIndex<[T]>>::get_unchecked_mut::precondition_check(_3, _4, move _7) -> [return: bb1, unwind unreachable];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ fn enumerated_loop(_1: &[T], _2: impl Fn(usize, &T)) -> () {
_7 = _4 as *mut T (PtrToPtr);
_8 = Offset(_7, _3);
StorageDead(_7);
_9 = move _8 as *const T (PointerCoercion(MutToConstPointer));
_9 = move _8 as *const T (PtrToPtr);
StorageDead(_8);
goto -> bb3;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ fn enumerated_loop(_1: &[T], _2: impl Fn(usize, &T)) -> () {
_7 = _4 as *mut T (PtrToPtr);
_8 = Offset(_7, _3);
StorageDead(_7);
_9 = move _8 as *const T (PointerCoercion(MutToConstPointer));
_9 = move _8 as *const T (PtrToPtr);
StorageDead(_8);
goto -> bb3;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ fn forward_loop(_1: &[T], _2: impl Fn(&T)) -> () {
_7 = _4 as *mut T (PtrToPtr);
_8 = Offset(_7, _3);
StorageDead(_7);
_9 = move _8 as *const T (PointerCoercion(MutToConstPointer));
_9 = move _8 as *const T (PtrToPtr);
StorageDead(_8);
goto -> bb3;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ fn forward_loop(_1: &[T], _2: impl Fn(&T)) -> () {
_7 = _4 as *mut T (PtrToPtr);
_8 = Offset(_7, _3);
StorageDead(_7);
_9 = move _8 as *const T (PointerCoercion(MutToConstPointer));
_9 = move _8 as *const T (PtrToPtr);
StorageDead(_8);
goto -> bb3;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ fn reverse_loop(_1: &[T], _2: impl Fn(&T)) -> () {
_7 = _4 as *mut T (PtrToPtr);
_8 = Offset(_7, _3);
StorageDead(_7);
_9 = move _8 as *const T (PointerCoercion(MutToConstPointer));
_9 = move _8 as *const T (PtrToPtr);
StorageDead(_8);
goto -> bb3;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ fn reverse_loop(_1: &[T], _2: impl Fn(&T)) -> () {
_7 = _4 as *mut T (PtrToPtr);
_8 = Offset(_7, _3);
StorageDead(_7);
_9 = move _8 as *const T (PointerCoercion(MutToConstPointer));
_9 = move _8 as *const T (PtrToPtr);
StorageDead(_8);
goto -> bb3;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ fn array_casts() -> () {
StorageLive(_4);
_4 = &mut _1;
_3 = &raw mut (*_4);
_2 = move _3 as *mut usize (PointerCoercion(ArrayToPointer));
_2 = move _3 as *mut usize (PtrToPtr);
StorageDead(_3);
StorageDead(_4);
StorageLive(_5);
Expand All @@ -87,7 +87,7 @@ fn array_casts() -> () {
StorageLive(_11);
_11 = &_8;
_10 = &raw const (*_11);
_9 = move _10 as *const usize (PointerCoercion(ArrayToPointer));
_9 = move _10 as *const usize (PtrToPtr);
StorageDead(_10);
StorageDead(_11);
StorageLive(_12);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ fn array_casts() -> () {
StorageLive(_4);
_4 = &mut _1;
_3 = &raw mut (*_4);
_2 = move _3 as *mut usize (PointerCoercion(ArrayToPointer));
_2 = move _3 as *mut usize (PtrToPtr);
StorageDead(_3);
StorageDead(_4);
StorageLive(_5);
Expand All @@ -87,7 +87,7 @@ fn array_casts() -> () {
StorageLive(_11);
_11 = &_8;
_10 = &raw const (*_11);
_9 = move _10 as *const usize (PointerCoercion(ArrayToPointer));
_9 = move _10 as *const usize (PtrToPtr);
StorageDead(_10);
StorageDead(_11);
StorageLive(_12);
Expand Down

0 comments on commit 3d5d7a2

Please sign in to comment.