Skip to content

Commit 4a7b6c0

Browse files
scottmcmscottmcm
committed
More GVN for PtrMetadata
`PtrMetadata` doesn't care about `*const`/`*mut`/`&`/`&mut`, so GVN away those casts in its argument. This includes updating MIR to allow calling PtrMetadata on references too, not just raw pointers. That means that `[T]::len` can be just `_0 = PtrMetadata(_1)`, for example. # Conflicts: # tests/mir-opt/pre-codegen/slice_index.slice_get_unchecked_mut_range.PreCodegen.after.panic-abort.mir # tests/mir-opt/pre-codegen/slice_index.slice_get_unchecked_mut_range.PreCodegen.after.panic-unwind.mir
1 parent 31d8696 commit 4a7b6c0

14 files changed

+319
-78
lines changed

‎compiler/rustc_codegen_ssa/src/mir/rvalue.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -639,7 +639,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
639639
(OperandValue::Immediate(llval), operand.layout)
640640
}
641641
mir::UnOp::PtrMetadata => {
642-
debug_assert!(operand.layout.ty.is_unsafe_ptr());
642+
debug_assert!(
643+
operand.layout.ty.is_unsafe_ptr() || operand.layout.ty.is_ref(),
644+
);
643645
let (_, meta) = operand.val.pointer_parts();
644646
assert_eq!(operand.layout.fields.count() > 1, meta.is_some());
645647
if let Some(meta) = meta {

‎compiler/rustc_const_eval/src/interpret/operator.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
460460
let res = ScalarInt::truncate_from_uint(res, layout.size).0;
461461
Ok(ImmTy::from_scalar(res.into(), layout))
462462
}
463-
ty::RawPtr(..) => {
463+
ty::RawPtr(..) | ty::Ref(..) => {
464464
assert_eq!(un_op, PtrMetadata);
465465
let (_, meta) = val.to_scalar_and_meta();
466466
Ok(match meta {

‎compiler/rustc_middle/src/mir/syntax.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1446,10 +1446,12 @@ pub enum UnOp {
14461446
Not,
14471447
/// The `-` operator for negation
14481448
Neg,
1449-
/// Get the metadata `M` from a `*const/mut impl Pointee<Metadata = M>`.
1449+
/// Gets the metadata `M` from a `*const`/`*mut`/`&`/`&mut` to
1450+
/// `impl Pointee<Metadata = M>`.
14501451
///
14511452
/// For example, this will give a `()` from `*const i32`, a `usize` from
1452-
/// `*mut [u8]`, or a pointer to a vtable from a `*const dyn Foo`.
1453+
/// `&mut [u8]`, or a `ptr::DynMetadata<dyn Foo>` (internally a pointer)
1454+
/// from a `*mut dyn Foo`.
14531455
///
14541456
/// Allowed only in [`MirPhase::Runtime`]; earlier it's an intrinsic.
14551457
PtrMetadata,

‎compiler/rustc_mir_transform/src/gvn.rs

Lines changed: 86 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -836,12 +836,8 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
836836
}
837837
Value::BinaryOp(op, lhs, rhs)
838838
}
839-
Rvalue::UnaryOp(op, ref mut arg) => {
840-
let arg = self.simplify_operand(arg, location)?;
841-
if let Some(value) = self.simplify_unary(op, arg) {
842-
return Some(value);
843-
}
844-
Value::UnaryOp(op, arg)
839+
Rvalue::UnaryOp(op, ref mut arg_op) => {
840+
return self.simplify_unary(op, arg_op, location);
845841
}
846842
Rvalue::Discriminant(ref mut place) => {
847843
let place = self.simplify_place_value(place, location)?;
@@ -971,8 +967,71 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
971967
}
972968

973969
#[instrument(level = "trace", skip(self), ret)]
974-
fn simplify_unary(&mut self, op: UnOp, value: VnIndex) -> Option<VnIndex> {
975-
let value = match (op, self.get(value)) {
970+
fn simplify_unary(
971+
&mut self,
972+
op: UnOp,
973+
arg_op: &mut Operand<'tcx>,
974+
location: Location,
975+
) -> Option<VnIndex> {
976+
let mut arg_index = self.simplify_operand(arg_op, location)?;
977+
978+
// PtrMetadata doesn't care about *const vs *mut vs & vs &mut,
979+
// so start by removing those distinctions so we can update the `Operand`
980+
if op == UnOp::PtrMetadata {
981+
let mut was_updated = false;
982+
loop {
983+
match self.get(arg_index) {
984+
// Pointer casts that preserve metadata, such as
985+
// `*const [i32]` <-> `*mut [i32]` <-> `*mut [f32]`.
986+
// It's critical that this not eliminate cases like
987+
// `*const [T]` -> `*const T` which remove metadata.
988+
// We run on potentially-generic MIR, though, so unlike codegen
989+
// we can't always know exactly what the metadata are.
990+
// Thankfully, equality on `ptr_metadata_ty_or_tail` gives us
991+
// what we need: `Ok(meta_ty)` if the metadata is known, or
992+
// `Err(tail_ty)` if not. Matching metadata is ok, but if
993+
// that's not known, then matching tail types is also ok,
994+
// allowing things like `*mut (?A, ?T)` <-> `*mut (?B, ?T)`.
995+
// FIXME: Would it be worth trying to normalize, rather than
996+
// passing the identity closure? Or are the types in the
997+
// Cast realistically about as normalized as we can get anyway?
998+
Value::Cast { kind: CastKind::PtrToPtr, value: inner, from, to }
999+
if from
1000+
.builtin_deref(true)
1001+
.unwrap()
1002+
.ptr_metadata_ty_or_tail(self.tcx, |t| t)
1003+
== to
1004+
.builtin_deref(true)
1005+
.unwrap()
1006+
.ptr_metadata_ty_or_tail(self.tcx, |t| t) =>
1007+
{
1008+
arg_index = *inner;
1009+
was_updated = true;
1010+
continue;
1011+
}
1012+
1013+
// `&mut *p`, `&raw *p`, etc don't change metadata.
1014+
Value::Address { place, kind: _, provenance: _ }
1015+
if let PlaceRef { local, projection: [PlaceElem::Deref] } =
1016+
place.as_ref()
1017+
&& let Some(local_index) = self.locals[local] =>
1018+
{
1019+
arg_index = local_index;
1020+
was_updated = true;
1021+
continue;
1022+
}
1023+
1024+
_ => {
1025+
if was_updated && let Some(op) = self.try_as_operand(arg_index, location) {
1026+
*arg_op = op;
1027+
}
1028+
break;
1029+
}
1030+
}
1031+
}
1032+
}
1033+
1034+
let value = match (op, self.get(arg_index)) {
9761035
(UnOp::Not, Value::UnaryOp(UnOp::Not, inner)) => return Some(*inner),
9771036
(UnOp::Neg, Value::UnaryOp(UnOp::Neg, inner)) => return Some(*inner),
9781037
(UnOp::Not, Value::BinaryOp(BinOp::Eq, lhs, rhs)) => {
@@ -984,9 +1043,26 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
9841043
(UnOp::PtrMetadata, Value::Aggregate(AggregateTy::RawPtr { .. }, _, fields)) => {
9851044
return Some(fields[1]);
9861045
}
987-
_ => return None,
1046+
// We have an unsizing cast, which assigns the length to fat pointer metadata.
1047+
(
1048+
UnOp::PtrMetadata,
1049+
Value::Cast {
1050+
kind: CastKind::PointerCoercion(ty::adjustment::PointerCoercion::Unsize),
1051+
from,
1052+
to,
1053+
..
1054+
},
1055+
) if let ty::Slice(..) = to.builtin_deref(true).unwrap().kind()
1056+
&& let ty::Array(_, len) = from.builtin_deref(true).unwrap().kind() =>
1057+
{
1058+
return self.insert_constant(Const::from_ty_const(
1059+
*len,
1060+
self.tcx.types.usize,
1061+
self.tcx,
1062+
));
1063+
}
1064+
_ => Value::UnaryOp(op, arg_index),
9881065
};
989-
9901066
Some(self.insert(value))
9911067
}
9921068

‎compiler/rustc_mir_transform/src/validate.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1116,12 +1116,17 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
11161116
UnOp::PtrMetadata => {
11171117
if !matches!(self.mir_phase, MirPhase::Runtime(_)) {
11181118
// It would probably be fine to support this in earlier phases,
1119-
// but at the time of writing it's only ever introduced from intrinsic lowering,
1119+
// but at the time of writing it's only ever introduced from intrinsic lowering
1120+
// or other runtime-phase optimization passes,
11201121
// so earlier things can just `bug!` on it.
11211122
self.fail(location, "PtrMetadata should be in runtime MIR only");
11221123
}
11231124

1124-
check_kinds!(a, "Cannot PtrMetadata non-pointer type {:?}", ty::RawPtr(..));
1125+
check_kinds!(
1126+
a,
1127+
"Cannot PtrMetadata non-pointer non-reference type {:?}",
1128+
ty::RawPtr(..) | ty::Ref(..)
1129+
);
11251130
}
11261131
}
11271132
}

‎tests/mir-opt/gvn.array_len.GVN.panic-abort.diff

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
- // MIR for `array_len` before GVN
2+
+ // MIR for `array_len` after GVN
3+
4+
fn array_len(_1: &mut [i32; 42]) -> usize {
5+
debug x => _1;
6+
let mut _0: usize;
7+
let _2: &[i32];
8+
let mut _3: &[i32; 42];
9+
let mut _4: *const [i32];
10+
scope 1 {
11+
debug x => _2;
12+
}
13+
14+
bb0: {
15+
- StorageLive(_2);
16+
+ nop;
17+
StorageLive(_3);
18+
_3 = &(*_1);
19+
_2 = move _3 as &[i32] (PointerCoercion(Unsize));
20+
StorageDead(_3);
21+
StorageLive(_4);
22+
_4 = &raw const (*_2);
23+
- _0 = PtrMetadata(move _4);
24+
+ _0 = const 42_usize;
25+
StorageDead(_4);
26+
- StorageDead(_2);
27+
+ nop;
28+
return;
29+
}
30+
}
31+

‎tests/mir-opt/gvn.array_len.GVN.panic-unwind.diff

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
- // MIR for `array_len` before GVN
2+
+ // MIR for `array_len` after GVN
3+
4+
fn array_len(_1: &mut [i32; 42]) -> usize {
5+
debug x => _1;
6+
let mut _0: usize;
7+
let _2: &[i32];
8+
let mut _3: &[i32; 42];
9+
let mut _4: *const [i32];
10+
scope 1 {
11+
debug x => _2;
12+
}
13+
14+
bb0: {
15+
- StorageLive(_2);
16+
+ nop;
17+
StorageLive(_3);
18+
_3 = &(*_1);
19+
_2 = move _3 as &[i32] (PointerCoercion(Unsize));
20+
StorageDead(_3);
21+
StorageLive(_4);
22+
_4 = &raw const (*_2);
23+
- _0 = PtrMetadata(move _4);
24+
+ _0 = const 42_usize;
25+
StorageDead(_4);
26+
- StorageDead(_2);
27+
+ nop;
28+
return;
29+
}
30+
}
31+

‎tests/mir-opt/gvn.fn_pointers.GVN.panic-abort.diff

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,18 @@
88
let mut _3: fn(u8) -> u8;
99
let _5: ();
1010
let mut _6: fn(u8) -> u8;
11-
let mut _9: {closure@$DIR/gvn.rs:612:19: 612:21};
11+
let mut _9: {closure@$DIR/gvn.rs:614:19: 614:21};
1212
let _10: ();
1313
let mut _11: fn();
14-
let mut _13: {closure@$DIR/gvn.rs:612:19: 612:21};
14+
let mut _13: {closure@$DIR/gvn.rs:614:19: 614:21};
1515
let _14: ();
1616
let mut _15: fn();
1717
scope 1 {
1818
debug f => _1;
1919
let _4: fn(u8) -> u8;
2020
scope 2 {
2121
debug g => _4;
22-
let _7: {closure@$DIR/gvn.rs:612:19: 612:21};
22+
let _7: {closure@$DIR/gvn.rs:614:19: 614:21};
2323
scope 3 {
2424
debug closure => _7;
2525
let _8: fn();
@@ -62,16 +62,16 @@
6262
StorageDead(_6);
6363
StorageDead(_5);
6464
- StorageLive(_7);
65-
- _7 = {closure@$DIR/gvn.rs:612:19: 612:21};
65+
- _7 = {closure@$DIR/gvn.rs:614:19: 614:21};
6666
- StorageLive(_8);
6767
+ nop;
68-
+ _7 = const ZeroSized: {closure@$DIR/gvn.rs:612:19: 612:21};
68+
+ _7 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21};
6969
+ nop;
7070
StorageLive(_9);
7171
- _9 = _7;
7272
- _8 = move _9 as fn() (PointerCoercion(ClosureFnPointer(Safe)));
73-
+ _9 = const ZeroSized: {closure@$DIR/gvn.rs:612:19: 612:21};
74-
+ _8 = const ZeroSized: {closure@$DIR/gvn.rs:612:19: 612:21} as fn() (PointerCoercion(ClosureFnPointer(Safe)));
73+
+ _9 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21};
74+
+ _8 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21} as fn() (PointerCoercion(ClosureFnPointer(Safe)));
7575
StorageDead(_9);
7676
StorageLive(_10);
7777
StorageLive(_11);
@@ -88,8 +88,8 @@
8888
StorageLive(_13);
8989
- _13 = _7;
9090
- _12 = move _13 as fn() (PointerCoercion(ClosureFnPointer(Safe)));
91-
+ _13 = const ZeroSized: {closure@$DIR/gvn.rs:612:19: 612:21};
92-
+ _12 = const ZeroSized: {closure@$DIR/gvn.rs:612:19: 612:21} as fn() (PointerCoercion(ClosureFnPointer(Safe)));
91+
+ _13 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21};
92+
+ _12 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21} as fn() (PointerCoercion(ClosureFnPointer(Safe)));
9393
StorageDead(_13);
9494
StorageLive(_14);
9595
StorageLive(_15);

‎tests/mir-opt/gvn.fn_pointers.GVN.panic-unwind.diff

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,18 @@
88
let mut _3: fn(u8) -> u8;
99
let _5: ();
1010
let mut _6: fn(u8) -> u8;
11-
let mut _9: {closure@$DIR/gvn.rs:612:19: 612:21};
11+
let mut _9: {closure@$DIR/gvn.rs:614:19: 614:21};
1212
let _10: ();
1313
let mut _11: fn();
14-
let mut _13: {closure@$DIR/gvn.rs:612:19: 612:21};
14+
let mut _13: {closure@$DIR/gvn.rs:614:19: 614:21};
1515
let _14: ();
1616
let mut _15: fn();
1717
scope 1 {
1818
debug f => _1;
1919
let _4: fn(u8) -> u8;
2020
scope 2 {
2121
debug g => _4;
22-
let _7: {closure@$DIR/gvn.rs:612:19: 612:21};
22+
let _7: {closure@$DIR/gvn.rs:614:19: 614:21};
2323
scope 3 {
2424
debug closure => _7;
2525
let _8: fn();
@@ -62,16 +62,16 @@
6262
StorageDead(_6);
6363
StorageDead(_5);
6464
- StorageLive(_7);
65-
- _7 = {closure@$DIR/gvn.rs:612:19: 612:21};
65+
- _7 = {closure@$DIR/gvn.rs:614:19: 614:21};
6666
- StorageLive(_8);
6767
+ nop;
68-
+ _7 = const ZeroSized: {closure@$DIR/gvn.rs:612:19: 612:21};
68+
+ _7 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21};
6969
+ nop;
7070
StorageLive(_9);
7171
- _9 = _7;
7272
- _8 = move _9 as fn() (PointerCoercion(ClosureFnPointer(Safe)));
73-
+ _9 = const ZeroSized: {closure@$DIR/gvn.rs:612:19: 612:21};
74-
+ _8 = const ZeroSized: {closure@$DIR/gvn.rs:612:19: 612:21} as fn() (PointerCoercion(ClosureFnPointer(Safe)));
73+
+ _9 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21};
74+
+ _8 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21} as fn() (PointerCoercion(ClosureFnPointer(Safe)));
7575
StorageDead(_9);
7676
StorageLive(_10);
7777
StorageLive(_11);
@@ -88,8 +88,8 @@
8888
StorageLive(_13);
8989
- _13 = _7;
9090
- _12 = move _13 as fn() (PointerCoercion(ClosureFnPointer(Safe)));
91-
+ _13 = const ZeroSized: {closure@$DIR/gvn.rs:612:19: 612:21};
92-
+ _12 = const ZeroSized: {closure@$DIR/gvn.rs:612:19: 612:21} as fn() (PointerCoercion(ClosureFnPointer(Safe)));
91+
+ _13 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21};
92+
+ _12 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21} as fn() (PointerCoercion(ClosureFnPointer(Safe)));
9393
StorageDead(_13);
9494
StorageLive(_14);
9595
StorageLive(_15);

‎tests/mir-opt/gvn.manual_slice_mut_len.GVN.panic-abort.diff

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
- // MIR for `manual_slice_mut_len` before GVN
2+
+ // MIR for `manual_slice_mut_len` after GVN
3+
4+
fn manual_slice_mut_len(_1: &mut [i32]) -> usize {
5+
debug x => _1;
6+
let mut _0: usize;
7+
let _2: *mut [i32];
8+
let mut _4: *mut [i32];
9+
let mut _5: *const [i32];
10+
scope 1 {
11+
debug x => _2;
12+
let _3: *const [i32];
13+
scope 2 {
14+
debug x => _3;
15+
}
16+
}
17+
18+
bb0: {
19+
- StorageLive(_2);
20+
+ nop;
21+
_2 = &raw mut (*_1);
22+
- StorageLive(_3);
23+
+ nop;
24+
StorageLive(_4);
25+
_4 = _2;
26+
- _3 = move _4 as *const [i32] (PtrToPtr);
27+
+ _3 = _2 as *const [i32] (PtrToPtr);
28+
StorageDead(_4);
29+
StorageLive(_5);
30+
_5 = _3;
31+
- _0 = PtrMetadata(move _5);
32+
+ _0 = PtrMetadata(_1);
33+
StorageDead(_5);
34+
- StorageDead(_3);
35+
- StorageDead(_2);
36+
+ nop;
37+
+ nop;
38+
return;
39+
}
40+
}
41+

0 commit comments

Comments
 (0)