Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 52 additions & 9 deletions compiler/rustc_mir_transform/src/gvn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1591,10 +1591,12 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
(Transmute, PtrToPtr) if self.pointers_have_same_metadata(from, to) => {
Some(Transmute)
}
// If would be legal to always do this, but we don't want to hide information
// It would be legal to always do this, but we don't want to hide information
// from the backend that it'd otherwise be able to use for optimizations.
(Transmute, Transmute)
if !self.type_may_have_niche_of_interest_to_backend(from) =>
if !self.transmute_may_have_niche_of_interest_to_backend(
inner_from, from, to,
) =>
{
Some(Transmute)
}
Expand Down Expand Up @@ -1642,24 +1644,65 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
}
}

/// Returns `false` if we know for sure that this type has no interesting niche,
/// and thus we can skip transmuting through it without worrying.
/// Returns `false` if we're confident that the middle type doesn't have an
/// interesting niche so we can skip that step when transmuting.
///
/// The backend will emit `assume`s when transmuting between types with niches,
/// so we want to preserve `i32 -> char -> u32` so that that data is around,
/// but it's fine to skip whole-range-is-value steps like `A -> u32 -> B`.
fn type_may_have_niche_of_interest_to_backend(&self, ty: Ty<'tcx>) -> bool {
let Ok(layout) = self.ecx.layout_of(ty) else {
fn transmute_may_have_niche_of_interest_to_backend(
&self,
from_ty: Ty<'tcx>,
middle_ty: Ty<'tcx>,
to_ty: Ty<'tcx>,
) -> bool {
let Ok(middle_layout) = self.ecx.layout_of(middle_ty) else {
// If it's too generic or something, then assume it might be interesting later.
return true;
};

if layout.uninhabited {
if middle_layout.uninhabited {
return true;
}

match layout.backend_repr {
BackendRepr::Scalar(a) => !a.is_always_valid(&self.ecx),
match middle_layout.backend_repr {
BackendRepr::Scalar(mid) => {
if mid.is_always_valid(&self.ecx) {
// With no niche it's never interesting, so don't bother
// looking at the layout of the other two types.
false
} else if let Ok(from_layout) = self.ecx.layout_of(from_ty)
&& !from_layout.uninhabited
&& from_layout.size == middle_layout.size
&& let BackendRepr::Scalar(from_a) = from_layout.backend_repr
&& let mid_range = mid.valid_range(&self.ecx)
&& let from_range = from_a.valid_range(&self.ecx)
&& mid_range.contains_range(from_range, middle_layout.size)
{
// The `from_range` is a (non-strict) subset of `mid_range`
// such as if we're doing `bool` -> `ascii::Char` -> `_`,
// where `from_range: 0..=1` and `mid_range: 0..=127`,
// and thus the middle doesn't tell us anything we don't
// already know from the initial type.
false
} else if let Ok(to_layout) = self.ecx.layout_of(to_ty)
&& !to_layout.uninhabited
&& to_layout.size == middle_layout.size
&& let BackendRepr::Scalar(to_a) = to_layout.backend_repr
&& let mid_range = mid.valid_range(&self.ecx)
&& let to_range = to_a.valid_range(&self.ecx)
&& mid_range.contains_range(to_range, middle_layout.size)
{
// The `to_range` is a (non-strict) subset of `mid_range`
// such as if we're doing `_` -> `ascii::Char` -> `bool`,
// where `mid_range: 0..=127` and `to_range: 0..=1`,
// and thus the middle doesn't tell us anything we don't
// already know from the final type.
false
Comment on lines 1674 to 1701
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mind commenting the logic? This looks good to me, but it took me a few frowns to understand, for instance why ranges are compared the way you wrote it

} else {
true
}
}
BackendRepr::ScalarPair(a, b) => {
!a.is_always_valid(&self.ecx) || !b.is_always_valid(&self.ecx)
}
Expand Down
14 changes: 11 additions & 3 deletions library/core/src/ptr/alignment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,12 @@ use crate::{cmp, fmt, hash, mem, num};
#[unstable(feature = "ptr_alignment_type", issue = "102070")]
#[derive(Copy, Clone, PartialEq, Eq)]
#[repr(transparent)]
pub struct Alignment(AlignmentEnum);
pub struct Alignment {
// This field is never used directly (nor is the enum),
// as it's just there to convey the validity invariant.
// (Hopefully it'll eventually be a pattern type instead.)
_inner_repr_trick: AlignmentEnum,
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this change be done in its own commit? This would make easier to understand what changes from the mir opt diff.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely. I split it to three:

  1. Just the compiler change to the mir-opt pass
  2. The change to Alignment::as_usize on its own
  3. The field change in Alignment

That way all the library changes are separated.


// Alignment is `repr(usize)`, but via extra steps.
const _: () = assert!(size_of::<Alignment>() == size_of::<usize>());
Expand All @@ -37,7 +42,7 @@ impl Alignment {
/// assert_eq!(Alignment::MIN.as_usize(), 1);
/// ```
#[unstable(feature = "ptr_alignment_type", issue = "102070")]
pub const MIN: Self = Self(AlignmentEnum::_Align1Shl0);
pub const MIN: Self = Self::new(1).unwrap();

/// Returns the alignment for a type.
///
Expand Down Expand Up @@ -166,7 +171,10 @@ impl Alignment {
#[unstable(feature = "ptr_alignment_type", issue = "102070")]
#[inline]
pub const fn as_usize(self) -> usize {
self.0 as usize
// Going through `as_nonzero` helps this be more clearly the inverse of
// `new_unchecked`, letting MIR optimizations fold it away.

self.as_nonzero().get()
}

/// Returns the alignment as a <code>[NonZero]<[usize]></code>.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,11 @@
scope 4 (inlined std::ptr::Unique::<[bool; 0]>::dangling) {
let mut _5: std::ptr::NonNull<[bool; 0]>;
scope 5 (inlined NonNull::<[bool; 0]>::dangling) {
let mut _6: std::num::NonZero<usize>;
scope 6 {
scope 8 (inlined std::ptr::Alignment::as_nonzero) {
}
scope 9 (inlined NonNull::<[bool; 0]>::without_provenance) {
let _7: *const [bool; 0];
let _6: *const [bool; 0];
scope 10 {
}
scope 11 (inlined NonZero::<usize>::get) {
Expand All @@ -45,11 +44,8 @@
StorageLive(_4);
StorageLive(_5);
StorageLive(_6);
_6 = const NonZero::<usize>(core::num::niche_types::NonZeroUsizeInner(1_usize));
StorageLive(_7);
_7 = const {0x1 as *const [bool; 0]};
_6 = const {0x1 as *const [bool; 0]};
_5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
StorageDead(_7);
StorageDead(_6);
_4 = const std::ptr::Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }};
StorageDead(_5);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,11 @@
scope 4 (inlined std::ptr::Unique::<[bool; 0]>::dangling) {
let mut _5: std::ptr::NonNull<[bool; 0]>;
scope 5 (inlined NonNull::<[bool; 0]>::dangling) {
let mut _6: std::num::NonZero<usize>;
scope 6 {
scope 8 (inlined std::ptr::Alignment::as_nonzero) {
}
scope 9 (inlined NonNull::<[bool; 0]>::without_provenance) {
let _7: *const [bool; 0];
let _6: *const [bool; 0];
scope 10 {
}
scope 11 (inlined NonZero::<usize>::get) {
Expand All @@ -45,11 +44,8 @@
StorageLive(_4);
StorageLive(_5);
StorageLive(_6);
_6 = const NonZero::<usize>(core::num::niche_types::NonZeroUsizeInner(1_usize));
StorageLive(_7);
_7 = const {0x1 as *const [bool; 0]};
_6 = const {0x1 as *const [bool; 0]};
_5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
StorageDead(_7);
StorageDead(_6);
_4 = const std::ptr::Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }};
StorageDead(_5);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,11 @@
scope 4 (inlined std::ptr::Unique::<[bool; 0]>::dangling) {
let mut _5: std::ptr::NonNull<[bool; 0]>;
scope 5 (inlined NonNull::<[bool; 0]>::dangling) {
let mut _6: std::num::NonZero<usize>;
scope 6 {
scope 8 (inlined std::ptr::Alignment::as_nonzero) {
}
scope 9 (inlined NonNull::<[bool; 0]>::without_provenance) {
let _7: *const [bool; 0];
let _6: *const [bool; 0];
scope 10 {
}
scope 11 (inlined NonZero::<usize>::get) {
Expand All @@ -45,11 +44,8 @@
StorageLive(_4);
StorageLive(_5);
StorageLive(_6);
_6 = const NonZero::<usize>(core::num::niche_types::NonZeroUsizeInner(1_usize));
StorageLive(_7);
_7 = const {0x1 as *const [bool; 0]};
_6 = const {0x1 as *const [bool; 0]};
_5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
StorageDead(_7);
StorageDead(_6);
_4 = const std::ptr::Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }};
StorageDead(_5);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,11 @@
scope 4 (inlined std::ptr::Unique::<[bool; 0]>::dangling) {
let mut _5: std::ptr::NonNull<[bool; 0]>;
scope 5 (inlined NonNull::<[bool; 0]>::dangling) {
let mut _6: std::num::NonZero<usize>;
scope 6 {
scope 8 (inlined std::ptr::Alignment::as_nonzero) {
}
scope 9 (inlined NonNull::<[bool; 0]>::without_provenance) {
let _7: *const [bool; 0];
let _6: *const [bool; 0];
scope 10 {
}
scope 11 (inlined NonZero::<usize>::get) {
Expand All @@ -45,11 +44,8 @@
StorageLive(_4);
StorageLive(_5);
StorageLive(_6);
_6 = const NonZero::<usize>(core::num::niche_types::NonZeroUsizeInner(1_usize));
StorageLive(_7);
_7 = const {0x1 as *const [bool; 0]};
_6 = const {0x1 as *const [bool; 0]};
_5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
StorageDead(_7);
StorageDead(_6);
_4 = const std::ptr::Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }};
StorageDead(_5);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,11 @@
scope 4 (inlined std::ptr::Unique::<[bool; 0]>::dangling) {
let mut _5: std::ptr::NonNull<[bool; 0]>;
scope 5 (inlined NonNull::<[bool; 0]>::dangling) {
let mut _6: std::num::NonZero<usize>;
scope 6 {
scope 8 (inlined std::ptr::Alignment::as_nonzero) {
}
scope 9 (inlined NonNull::<[bool; 0]>::without_provenance) {
let _7: *const [bool; 0];
let _6: *const [bool; 0];
scope 10 {
}
scope 11 (inlined NonZero::<usize>::get) {
Expand All @@ -45,14 +44,10 @@
StorageLive(_4);
StorageLive(_5);
StorageLive(_6);
- _6 = const std::ptr::Alignment::of::<[bool; 0]>::{constant#0} as std::num::NonZero<usize> (Transmute);
+ _6 = const NonZero::<usize>(core::num::niche_types::NonZeroUsizeInner(1_usize));
StorageLive(_7);
- _7 = copy _6 as *const [bool; 0] (Transmute);
- _5 = NonNull::<[bool; 0]> { pointer: copy _7 };
+ _7 = const {0x1 as *const [bool; 0]};
- _6 = const std::ptr::Alignment::of::<[bool; 0]>::{constant#0} as *const [bool; 0] (Transmute);
- _5 = NonNull::<[bool; 0]> { pointer: copy _6 };
+ _6 = const {0x1 as *const [bool; 0]};
+ _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
StorageDead(_7);
StorageDead(_6);
- _4 = std::ptr::Unique::<[bool; 0]> { pointer: move _5, _marker: const PhantomData::<[bool; 0]> };
+ _4 = const std::ptr::Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,11 @@
scope 4 (inlined std::ptr::Unique::<[bool; 0]>::dangling) {
let mut _5: std::ptr::NonNull<[bool; 0]>;
scope 5 (inlined NonNull::<[bool; 0]>::dangling) {
let mut _6: std::num::NonZero<usize>;
scope 6 {
scope 8 (inlined std::ptr::Alignment::as_nonzero) {
}
scope 9 (inlined NonNull::<[bool; 0]>::without_provenance) {
let _7: *const [bool; 0];
let _6: *const [bool; 0];
scope 10 {
}
scope 11 (inlined NonZero::<usize>::get) {
Expand All @@ -45,14 +44,10 @@
StorageLive(_4);
StorageLive(_5);
StorageLive(_6);
- _6 = const std::ptr::Alignment::of::<[bool; 0]>::{constant#0} as std::num::NonZero<usize> (Transmute);
+ _6 = const NonZero::<usize>(core::num::niche_types::NonZeroUsizeInner(1_usize));
StorageLive(_7);
- _7 = copy _6 as *const [bool; 0] (Transmute);
- _5 = NonNull::<[bool; 0]> { pointer: copy _7 };
+ _7 = const {0x1 as *const [bool; 0]};
- _6 = const std::ptr::Alignment::of::<[bool; 0]>::{constant#0} as *const [bool; 0] (Transmute);
- _5 = NonNull::<[bool; 0]> { pointer: copy _6 };
+ _6 = const {0x1 as *const [bool; 0]};
+ _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
StorageDead(_7);
StorageDead(_6);
- _4 = std::ptr::Unique::<[bool; 0]> { pointer: move _5, _marker: const PhantomData::<[bool; 0]> };
+ _4 = const std::ptr::Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,11 @@
scope 4 (inlined std::ptr::Unique::<[bool; 0]>::dangling) {
let mut _5: std::ptr::NonNull<[bool; 0]>;
scope 5 (inlined NonNull::<[bool; 0]>::dangling) {
let mut _6: std::num::NonZero<usize>;
scope 6 {
scope 8 (inlined std::ptr::Alignment::as_nonzero) {
}
scope 9 (inlined NonNull::<[bool; 0]>::without_provenance) {
let _7: *const [bool; 0];
let _6: *const [bool; 0];
scope 10 {
}
scope 11 (inlined NonZero::<usize>::get) {
Expand All @@ -45,14 +44,10 @@
StorageLive(_4);
StorageLive(_5);
StorageLive(_6);
- _6 = const std::ptr::Alignment::of::<[bool; 0]>::{constant#0} as std::num::NonZero<usize> (Transmute);
+ _6 = const NonZero::<usize>(core::num::niche_types::NonZeroUsizeInner(1_usize));
StorageLive(_7);
- _7 = copy _6 as *const [bool; 0] (Transmute);
- _5 = NonNull::<[bool; 0]> { pointer: copy _7 };
+ _7 = const {0x1 as *const [bool; 0]};
- _6 = const std::ptr::Alignment::of::<[bool; 0]>::{constant#0} as *const [bool; 0] (Transmute);
- _5 = NonNull::<[bool; 0]> { pointer: copy _6 };
+ _6 = const {0x1 as *const [bool; 0]};
+ _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
StorageDead(_7);
StorageDead(_6);
- _4 = std::ptr::Unique::<[bool; 0]> { pointer: move _5, _marker: const PhantomData::<[bool; 0]> };
+ _4 = const std::ptr::Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,11 @@
scope 4 (inlined std::ptr::Unique::<[bool; 0]>::dangling) {
let mut _5: std::ptr::NonNull<[bool; 0]>;
scope 5 (inlined NonNull::<[bool; 0]>::dangling) {
let mut _6: std::num::NonZero<usize>;
scope 6 {
scope 8 (inlined std::ptr::Alignment::as_nonzero) {
}
scope 9 (inlined NonNull::<[bool; 0]>::without_provenance) {
let _7: *const [bool; 0];
let _6: *const [bool; 0];
scope 10 {
}
scope 11 (inlined NonZero::<usize>::get) {
Expand All @@ -45,14 +44,10 @@
StorageLive(_4);
StorageLive(_5);
StorageLive(_6);
- _6 = const std::ptr::Alignment::of::<[bool; 0]>::{constant#0} as std::num::NonZero<usize> (Transmute);
+ _6 = const NonZero::<usize>(core::num::niche_types::NonZeroUsizeInner(1_usize));
StorageLive(_7);
- _7 = copy _6 as *const [bool; 0] (Transmute);
- _5 = NonNull::<[bool; 0]> { pointer: copy _7 };
+ _7 = const {0x1 as *const [bool; 0]};
- _6 = const std::ptr::Alignment::of::<[bool; 0]>::{constant#0} as *const [bool; 0] (Transmute);
- _5 = NonNull::<[bool; 0]> { pointer: copy _6 };
+ _6 = const {0x1 as *const [bool; 0]};
+ _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
StorageDead(_7);
StorageDead(_6);
- _4 = std::ptr::Unique::<[bool; 0]> { pointer: move _5, _marker: const PhantomData::<[bool; 0]> };
+ _4 = const std::ptr::Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }};
Expand Down
Loading
Loading