-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add an intrinsic for ptr::from_raw_parts(_mut)
#123840
Changes from all commits
5800dc1
b76faff
70df9d9
e6b2b76
4f44426
de64ff7
9520ceb
5e1d16c
bb8d6f7
1398fe7
5e785b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1351,6 +1351,21 @@ pub enum AggregateKind<'tcx> { | |
Closure(DefId, GenericArgsRef<'tcx>), | ||
Coroutine(DefId, GenericArgsRef<'tcx>), | ||
CoroutineClosure(DefId, GenericArgsRef<'tcx>), | ||
|
||
/// Construct a raw pointer from the data pointer and metadata. | ||
/// | ||
/// The `Ty` here is the type of the *pointee*, not the pointer itself. | ||
/// The `Mutability` indicates whether this produces a `*const` or `*mut`. | ||
/// | ||
/// The [`Rvalue::Aggregate`] operands for thus must be | ||
/// | ||
/// 0. A raw pointer of matching mutability with any [`core::ptr::Thin`] pointee | ||
/// 1. A value of the appropriate [`core::ptr::Pointee::Metadata`] type | ||
/// | ||
/// *Both* operands must always be included, even the unit value if this is | ||
/// creating a thin pointer. If you're just converting between thin pointers, | ||
/// you may want an [`Rvalue::Cast`] with [`CastKind::PtrToPtr`] instead. | ||
RawPtr(Ty<'tcx>, Mutability), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In terms of generality, should this support only raw pointers, or borrows too? Or to be left as a follow-up experiment? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right now I think that It's also nice in the library for the intrinsic to be able to be safe, which it can't if it makes references. That could be done with separate intrinsics so the raw pointer one can still be safe, but that's again making the size of the change bigger again, so I think not doing it in this PR, at least, is best. |
||
} | ||
|
||
#[derive(Copy, Clone, Debug, PartialEq, Eq, TyEncodable, TyDecodable, Hash, HashStable)] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -885,6 +885,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { | |
AggregateKind::Adt(did, ..) => tcx.def_kind(did) != DefKind::Enum, | ||
// Coroutines are never ZST, as they at least contain the implicit states. | ||
AggregateKind::Coroutine(..) => false, | ||
AggregateKind::RawPtr(..) => bug!("MIR for RawPtr aggregate must have 2 fields"), | ||
}; | ||
|
||
if is_zst { | ||
|
@@ -910,6 +911,8 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { | |
} | ||
// Do not track unions. | ||
AggregateKind::Adt(_, _, _, _, Some(_)) => return None, | ||
// FIXME: Do the extra work to GVN `from_raw_parts` | ||
AggregateKind::RawPtr(..) => return None, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I started down the path of an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok. |
||
}; | ||
|
||
let fields: Option<Vec<_>> = fields | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,6 +36,7 @@ impl<'tcx> MirPass<'tcx> for InstSimplify { | |
ctx.simplify_bool_cmp(&statement.source_info, rvalue); | ||
ctx.simplify_ref_deref(&statement.source_info, rvalue); | ||
ctx.simplify_len(&statement.source_info, rvalue); | ||
ctx.simplify_ptr_aggregate(&statement.source_info, rvalue); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could the new opt be in a standalone commit, to see its effect on tests? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a new mir-opt test for |
||
ctx.simplify_cast(rvalue); | ||
} | ||
_ => {} | ||
|
@@ -58,8 +59,17 @@ struct InstSimplifyContext<'tcx, 'a> { | |
|
||
impl<'tcx> InstSimplifyContext<'tcx, '_> { | ||
fn should_simplify(&self, source_info: &SourceInfo, rvalue: &Rvalue<'tcx>) -> bool { | ||
self.should_simplify_custom(source_info, "Rvalue", rvalue) | ||
} | ||
|
||
fn should_simplify_custom( | ||
&self, | ||
source_info: &SourceInfo, | ||
label: &str, | ||
value: impl std::fmt::Debug, | ||
) -> bool { | ||
self.tcx.consider_optimizing(|| { | ||
format!("InstSimplify - Rvalue: {rvalue:?} SourceInfo: {source_info:?}") | ||
format!("InstSimplify - {label}: {value:?} SourceInfo: {source_info:?}") | ||
}) | ||
} | ||
|
||
|
@@ -111,7 +121,7 @@ impl<'tcx> InstSimplifyContext<'tcx, '_> { | |
if a.const_.ty().is_bool() { a.const_.try_to_bool() } else { None } | ||
} | ||
|
||
/// Transform "&(*a)" ==> "a". | ||
/// Transform `&(*a)` ==> `a`. | ||
fn simplify_ref_deref(&self, source_info: &SourceInfo, rvalue: &mut Rvalue<'tcx>) { | ||
if let Rvalue::Ref(_, _, place) = rvalue { | ||
if let Some((base, ProjectionElem::Deref)) = place.as_ref().last_projection() { | ||
|
@@ -131,7 +141,7 @@ impl<'tcx> InstSimplifyContext<'tcx, '_> { | |
} | ||
} | ||
|
||
/// Transform "Len([_; N])" ==> "N". | ||
/// Transform `Len([_; N])` ==> `N`. | ||
fn simplify_len(&self, source_info: &SourceInfo, rvalue: &mut Rvalue<'tcx>) { | ||
if let Rvalue::Len(ref place) = *rvalue { | ||
let place_ty = place.ty(self.local_decls, self.tcx).ty; | ||
|
@@ -147,6 +157,30 @@ impl<'tcx> InstSimplifyContext<'tcx, '_> { | |
} | ||
} | ||
|
||
/// Transform `Aggregate(RawPtr, [p, ()])` ==> `Cast(PtrToPtr, p)`. | ||
fn simplify_ptr_aggregate(&self, source_info: &SourceInfo, rvalue: &mut Rvalue<'tcx>) { | ||
if let Rvalue::Aggregate(box AggregateKind::RawPtr(pointee_ty, mutability), fields) = rvalue | ||
{ | ||
let meta_ty = fields.raw[1].ty(self.local_decls, self.tcx); | ||
if meta_ty.is_unit() { | ||
// The mutable borrows we're holding prevent printing `rvalue` here | ||
if !self.should_simplify_custom( | ||
source_info, | ||
"Aggregate::RawPtr", | ||
(&pointee_ty, *mutability, &fields), | ||
) { | ||
return; | ||
} | ||
|
||
let mut fields = std::mem::take(fields); | ||
let _meta = fields.pop().unwrap(); | ||
let data = fields.pop().unwrap(); | ||
let ptr_ty = Ty::new_ptr(self.tcx, *pointee_ty, *mutability); | ||
*rvalue = Rvalue::Cast(CastKind::PtrToPtr, data, ptr_ty); | ||
} | ||
} | ||
} | ||
|
||
fn simplify_ub_check(&self, source_info: &SourceInfo, rvalue: &mut Rvalue<'tcx>) { | ||
if let Rvalue::NullaryOp(NullOp::UbChecks, _) = *rvalue { | ||
let const_ = Const::from_bool(self.tcx, self.tcx.sess.ub_checks()); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: should this appear in an earlier commit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first few commits here aren't split up super great, just checkpoints where it built and the tests pass. The tests "passing" for them wasn't terribly meaningful, though, since without an intrinsic for it and without the lowering using it, then the code wasn't actually ever running.
LMK if you're fine leaving it as-is or would rather I just merge the first few.