Skip to content

Commit d7c5937

Browse files
committed
Auto merge of rust-lang#126844 - scottmcm:more-ptr-cast-gvn, r=saethlin
Remove more `PtrToPtr` casts in GVN This addresses two things I noticed in MIR: 1. `NonNull::<T>::eq` does `(a as *mut T) == (b as *mut T)`, but it could just compare the `*const T`s, so this removes `PtrToPtr` casts that are on both sides of a pointer comparison, so long as they're not fat-to-thin casts. 2. `NonNull::<T>::addr` does `transmute::<_, usize>(p as *const ())`, but so long as `T: Thin` that cast doesn't do anything, and thus we can directly transmute the `*const T` instead. r? mir-opt
2 parents 4bdf8d2 + dd545e1 commit d7c5937

23 files changed

+1573
-160
lines changed

compiler/rustc_middle/src/mir/tcx.rs

+1-13
Original file line numberDiff line numberDiff line change
@@ -289,19 +289,7 @@ impl<'tcx> UnOp {
289289
pub fn ty(&self, tcx: TyCtxt<'tcx>, arg_ty: Ty<'tcx>) -> Ty<'tcx> {
290290
match self {
291291
UnOp::Not | UnOp::Neg => arg_ty,
292-
UnOp::PtrMetadata => {
293-
let pointee_ty = arg_ty
294-
.builtin_deref(true)
295-
.unwrap_or_else(|| bug!("PtrMetadata of non-dereferenceable ty {arg_ty:?}"));
296-
if pointee_ty.is_trivially_sized(tcx) {
297-
tcx.types.unit
298-
} else {
299-
let Some(metadata_def_id) = tcx.lang_items().metadata_type() else {
300-
bug!("No metadata_type lang item while looking at {arg_ty:?}")
301-
};
302-
Ty::new_projection(tcx, metadata_def_id, [pointee_ty])
303-
}
304-
}
292+
UnOp::PtrMetadata => arg_ty.pointee_metadata_ty_or_projection(tcx),
305293
}
306294
}
307295
}

compiler/rustc_middle/src/ty/sty.rs

+28
Original file line numberDiff line numberDiff line change
@@ -1647,6 +1647,34 @@ impl<'tcx> Ty<'tcx> {
16471647
}
16481648
}
16491649

1650+
/// Given a pointer or reference type, returns the type of the *pointee*'s
1651+
/// metadata. If it can't be determined exactly (perhaps due to still
1652+
/// being generic) then a projection through `ptr::Pointee` will be returned.
1653+
///
1654+
/// This is particularly useful for getting the type of the result of
1655+
/// [`UnOp::PtrMetadata`](crate::mir::UnOp::PtrMetadata).
1656+
///
1657+
/// Panics if `self` is not dereferencable.
1658+
#[track_caller]
1659+
pub fn pointee_metadata_ty_or_projection(self, tcx: TyCtxt<'tcx>) -> Ty<'tcx> {
1660+
let Some(pointee_ty) = self.builtin_deref(true) else {
1661+
bug!("Type {self:?} is not a pointer or reference type")
1662+
};
1663+
if pointee_ty.is_trivially_sized(tcx) {
1664+
tcx.types.unit
1665+
} else {
1666+
match pointee_ty.ptr_metadata_ty_or_tail(tcx, |x| x) {
1667+
Ok(metadata_ty) => metadata_ty,
1668+
Err(tail_ty) => {
1669+
let Some(metadata_def_id) = tcx.lang_items().metadata_type() else {
1670+
bug!("No metadata_type lang item while looking at {self:?}")
1671+
};
1672+
Ty::new_projection(tcx, metadata_def_id, [tail_ty])
1673+
}
1674+
}
1675+
}
1676+
}
1677+
16501678
/// When we create a closure, we record its kind (i.e., what trait
16511679
/// it implements, constrained by how it uses its borrows) into its
16521680
/// [`ty::ClosureArgs`] or [`ty::CoroutineClosureArgs`] using a type

compiler/rustc_mir_transform/src/cost_checker.rs

+22-7
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,15 @@ impl<'tcx> Visitor<'tcx> for CostChecker<'_, 'tcx> {
6060

6161
fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, _location: Location) {
6262
match rvalue {
63-
Rvalue::NullaryOp(NullOp::UbChecks, ..) if !self.tcx.sess.ub_checks() => {
63+
Rvalue::NullaryOp(NullOp::UbChecks, ..)
64+
if !self
65+
.tcx
66+
.sess
67+
.opts
68+
.unstable_opts
69+
.inline_mir_preserve_debug
70+
.unwrap_or(self.tcx.sess.ub_checks()) =>
71+
{
6472
// If this is in optimized MIR it's because it's used later,
6573
// so if we don't need UB checks this session, give a bonus
6674
// here to offset the cost of the call later.
@@ -111,12 +119,19 @@ impl<'tcx> Visitor<'tcx> for CostChecker<'_, 'tcx> {
111119
}
112120
}
113121
TerminatorKind::Assert { unwind, msg, .. } => {
114-
self.penalty +=
115-
if msg.is_optional_overflow_check() && !self.tcx.sess.overflow_checks() {
116-
INSTR_COST
117-
} else {
118-
CALL_PENALTY
119-
};
122+
self.penalty += if msg.is_optional_overflow_check()
123+
&& !self
124+
.tcx
125+
.sess
126+
.opts
127+
.unstable_opts
128+
.inline_mir_preserve_debug
129+
.unwrap_or(self.tcx.sess.overflow_checks())
130+
{
131+
INSTR_COST
132+
} else {
133+
CALL_PENALTY
134+
};
120135
if let UnwindAction::Cleanup(_) = unwind {
121136
self.penalty += LANDINGPAD_PENALTY;
122137
}

compiler/rustc_mir_transform/src/gvn.rs

+85-29
Original file line numberDiff line numberDiff line change
@@ -823,18 +823,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
823823
return self.simplify_cast(kind, value, to, location);
824824
}
825825
Rvalue::BinaryOp(op, box (ref mut lhs, ref mut rhs)) => {
826-
let ty = lhs.ty(self.local_decls, self.tcx);
827-
let lhs = self.simplify_operand(lhs, location);
828-
let rhs = self.simplify_operand(rhs, location);
829-
// Only short-circuit options after we called `simplify_operand`
830-
// on both operands for side effect.
831-
let lhs = lhs?;
832-
let rhs = rhs?;
833-
834-
if let Some(value) = self.simplify_binary(op, ty, lhs, rhs) {
835-
return Some(value);
836-
}
837-
Value::BinaryOp(op, lhs, rhs)
826+
return self.simplify_binary(op, lhs, rhs, location);
838827
}
839828
Rvalue::UnaryOp(op, ref mut arg_op) => {
840829
return self.simplify_unary(op, arg_op, location);
@@ -987,23 +976,10 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
987976
// `*const [T]` -> `*const T` which remove metadata.
988977
// We run on potentially-generic MIR, though, so unlike codegen
989978
// 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?
979+
// To allow things like `*mut (?A, ?T)` <-> `*mut (?B, ?T)`,
980+
// it's fine to get a projection as the type.
998981
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) =>
982+
if self.pointers_have_same_metadata(*from, *to) =>
1007983
{
1008984
arg_index = *inner;
1009985
was_updated = true;
@@ -1068,6 +1044,52 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
10681044

10691045
#[instrument(level = "trace", skip(self), ret)]
10701046
fn simplify_binary(
1047+
&mut self,
1048+
op: BinOp,
1049+
lhs_operand: &mut Operand<'tcx>,
1050+
rhs_operand: &mut Operand<'tcx>,
1051+
location: Location,
1052+
) -> Option<VnIndex> {
1053+
let lhs = self.simplify_operand(lhs_operand, location);
1054+
let rhs = self.simplify_operand(rhs_operand, location);
1055+
// Only short-circuit options after we called `simplify_operand`
1056+
// on both operands for side effect.
1057+
let mut lhs = lhs?;
1058+
let mut rhs = rhs?;
1059+
1060+
let lhs_ty = lhs_operand.ty(self.local_decls, self.tcx);
1061+
1062+
// If we're comparing pointers, remove `PtrToPtr` casts if the from
1063+
// types of both casts and the metadata all match.
1064+
if let BinOp::Eq | BinOp::Ne | BinOp::Lt | BinOp::Le | BinOp::Gt | BinOp::Ge = op
1065+
&& lhs_ty.is_any_ptr()
1066+
&& let Value::Cast {
1067+
kind: CastKind::PtrToPtr, value: lhs_value, from: lhs_from, ..
1068+
} = self.get(lhs)
1069+
&& let Value::Cast {
1070+
kind: CastKind::PtrToPtr, value: rhs_value, from: rhs_from, ..
1071+
} = self.get(rhs)
1072+
&& lhs_from == rhs_from
1073+
&& self.pointers_have_same_metadata(*lhs_from, lhs_ty)
1074+
{
1075+
lhs = *lhs_value;
1076+
rhs = *rhs_value;
1077+
if let Some(op) = self.try_as_operand(lhs, location) {
1078+
*lhs_operand = op;
1079+
}
1080+
if let Some(op) = self.try_as_operand(rhs, location) {
1081+
*rhs_operand = op;
1082+
}
1083+
}
1084+
1085+
if let Some(value) = self.simplify_binary_inner(op, lhs_ty, lhs, rhs) {
1086+
return Some(value);
1087+
}
1088+
let value = Value::BinaryOp(op, lhs, rhs);
1089+
Some(self.insert(value))
1090+
}
1091+
1092+
fn simplify_binary_inner(
10711093
&mut self,
10721094
op: BinOp,
10731095
lhs_ty: Ty<'tcx>,
@@ -1228,14 +1250,33 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
12281250
}
12291251
}
12301252

1253+
// PtrToPtr-then-PtrToPtr can skip the intermediate step
12311254
if let PtrToPtr = kind
12321255
&& let Value::Cast { kind: inner_kind, value: inner_value, from: inner_from, to: _ } =
12331256
*self.get(value)
12341257
&& let PtrToPtr = inner_kind
12351258
{
12361259
from = inner_from;
12371260
value = inner_value;
1238-
*kind = PtrToPtr;
1261+
was_updated = true;
1262+
if inner_from == to {
1263+
return Some(inner_value);
1264+
}
1265+
}
1266+
1267+
// PtrToPtr-then-Transmute can just transmute the original, so long as the
1268+
// PtrToPtr didn't change metadata (and thus the size of the pointer)
1269+
if let Transmute = kind
1270+
&& let Value::Cast {
1271+
kind: PtrToPtr,
1272+
value: inner_value,
1273+
from: inner_from,
1274+
to: inner_to,
1275+
} = *self.get(value)
1276+
&& self.pointers_have_same_metadata(inner_from, inner_to)
1277+
{
1278+
from = inner_from;
1279+
value = inner_value;
12391280
was_updated = true;
12401281
if inner_from == to {
12411282
return Some(inner_value);
@@ -1289,6 +1330,21 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
12891330
// Fallback: a symbolic `Len`.
12901331
Some(self.insert(Value::Len(inner)))
12911332
}
1333+
1334+
fn pointers_have_same_metadata(&self, left_ptr_ty: Ty<'tcx>, right_ptr_ty: Ty<'tcx>) -> bool {
1335+
let left_meta_ty = left_ptr_ty.pointee_metadata_ty_or_projection(self.tcx);
1336+
let right_meta_ty = right_ptr_ty.pointee_metadata_ty_or_projection(self.tcx);
1337+
if left_meta_ty == right_meta_ty {
1338+
true
1339+
} else if let Ok(left) =
1340+
self.tcx.try_normalize_erasing_regions(self.param_env, left_meta_ty)
1341+
&& let Ok(right) = self.tcx.try_normalize_erasing_regions(self.param_env, right_meta_ty)
1342+
{
1343+
left == right
1344+
} else {
1345+
false
1346+
}
1347+
}
12921348
}
12931349

12941350
fn op_to_prop_const<'tcx>(

tests/coverage/closure_macro.cov-map

+4-4
Original file line numberDiff line numberDiff line change
@@ -22,19 +22,19 @@ Number of file 0 mappings: 5
2222
- Code(Counter(0)) at (prev + 3, 1) to (start + 0, 2)
2323

2424
Function name: closure_macro::main::{closure#0}
25-
Raw bytes (35): 0x[01, 01, 03, 01, 05, 05, 0b, 09, 0d, 05, 01, 10, 1c, 03, 21, 05, 04, 11, 01, 27, 02, 03, 11, 00, 16, 0d, 00, 17, 00, 1e, 07, 02, 09, 00, 0a]
25+
Raw bytes (35): 0x[01, 01, 03, 01, 05, 05, 0b, 09, 00, 05, 01, 10, 1c, 03, 21, 05, 04, 11, 01, 27, 02, 03, 11, 00, 16, 00, 00, 17, 00, 1e, 07, 02, 09, 00, 0a]
2626
Number of files: 1
2727
- file 0 => global file 1
2828
Number of expressions: 3
2929
- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
3030
- expression 1 operands: lhs = Counter(1), rhs = Expression(2, Add)
31-
- expression 2 operands: lhs = Counter(2), rhs = Counter(3)
31+
- expression 2 operands: lhs = Counter(2), rhs = Zero
3232
Number of file 0 mappings: 5
3333
- Code(Counter(0)) at (prev + 16, 28) to (start + 3, 33)
3434
- Code(Counter(1)) at (prev + 4, 17) to (start + 1, 39)
3535
- Code(Expression(0, Sub)) at (prev + 3, 17) to (start + 0, 22)
3636
= (c0 - c1)
37-
- Code(Counter(3)) at (prev + 0, 23) to (start + 0, 30)
37+
- Code(Zero) at (prev + 0, 23) to (start + 0, 30)
3838
- Code(Expression(1, Add)) at (prev + 2, 9) to (start + 0, 10)
39-
= (c1 + (c2 + c3))
39+
= (c1 + (c2 + Zero))
4040

tests/coverage/closure_macro_async.cov-map

+4-4
Original file line numberDiff line numberDiff line change
@@ -30,19 +30,19 @@ Number of file 0 mappings: 5
3030
- Code(Counter(0)) at (prev + 3, 1) to (start + 0, 2)
3131

3232
Function name: closure_macro_async::test::{closure#0}::{closure#0}
33-
Raw bytes (35): 0x[01, 01, 03, 01, 05, 05, 0b, 09, 0d, 05, 01, 12, 1c, 03, 21, 05, 04, 11, 01, 27, 02, 03, 11, 00, 16, 0d, 00, 17, 00, 1e, 07, 02, 09, 00, 0a]
33+
Raw bytes (35): 0x[01, 01, 03, 01, 05, 05, 0b, 09, 00, 05, 01, 12, 1c, 03, 21, 05, 04, 11, 01, 27, 02, 03, 11, 00, 16, 00, 00, 17, 00, 1e, 07, 02, 09, 00, 0a]
3434
Number of files: 1
3535
- file 0 => global file 1
3636
Number of expressions: 3
3737
- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
3838
- expression 1 operands: lhs = Counter(1), rhs = Expression(2, Add)
39-
- expression 2 operands: lhs = Counter(2), rhs = Counter(3)
39+
- expression 2 operands: lhs = Counter(2), rhs = Zero
4040
Number of file 0 mappings: 5
4141
- Code(Counter(0)) at (prev + 18, 28) to (start + 3, 33)
4242
- Code(Counter(1)) at (prev + 4, 17) to (start + 1, 39)
4343
- Code(Expression(0, Sub)) at (prev + 3, 17) to (start + 0, 22)
4444
= (c0 - c1)
45-
- Code(Counter(3)) at (prev + 0, 23) to (start + 0, 30)
45+
- Code(Zero) at (prev + 0, 23) to (start + 0, 30)
4646
- Code(Expression(1, Add)) at (prev + 2, 9) to (start + 0, 10)
47-
= (c1 + (c2 + c3))
47+
= (c1 + (c2 + Zero))
4848

0 commit comments

Comments
 (0)