Skip to content

Use custom wrap-around type instead of RangeInclusive #88242

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

Merged
merged 12 commits into from
Aug 25, 2021
3 changes: 1 addition & 2 deletions compiler/rustc_codegen_llvm/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -462,15 +462,14 @@ impl BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
load: &'ll Value,
scalar: &abi::Scalar,
) {
let vr = scalar.valid_range.clone();
match scalar.value {
abi::Int(..) => {
let range = scalar.valid_range_exclusive(bx);
if range.start != range.end {
bx.range_metadata(load, range);
}
}
abi::Pointer if vr.start() < vr.end() && !vr.contains(&0) => {
abi::Pointer if !scalar.valid_range.contains_zero() => {
bx.nonnull_metadata(load);
}
_ => {}
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_codegen_llvm/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ use rustc_middle::mir::interpret::{
use rustc_middle::mir::mono::MonoItem;
use rustc_middle::ty::{self, Instance, Ty};
use rustc_middle::{bug, span_bug};
use rustc_target::abi::{AddressSpace, Align, HasDataLayout, LayoutOf, Primitive, Scalar, Size};
use rustc_target::abi::{
AddressSpace, Align, HasDataLayout, LayoutOf, Primitive, Scalar, Size, WrappingRange,
};
use tracing::debug;

pub fn const_alloc_to_llvm(cx: &CodegenCx<'ll, '_>, alloc: &Allocation) -> &'ll Value {
Expand Down Expand Up @@ -59,7 +61,7 @@ pub fn const_alloc_to_llvm(cx: &CodegenCx<'ll, '_>, alloc: &Allocation) -> &'ll
Pointer::new(alloc_id, Size::from_bytes(ptr_offset)),
&cx.tcx,
),
&Scalar { value: Primitive::Pointer, valid_range: 0..=!0 },
&Scalar { value: Primitive::Pointer, valid_range: WrappingRange { start: 0, end: !0 } },
cx.type_i8p_ext(address_space),
));
next_offset = offset + pointer_size;
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,11 +406,11 @@ fn push_debuginfo_type_name<'tcx>(
let dataful_discriminant_range =
&dataful_variant_layout.largest_niche.as_ref().unwrap().scalar.valid_range;

let min = dataful_discriminant_range.start();
let min = tag.value.size(&tcx).truncate(*min);
let min = dataful_discriminant_range.start;
let min = tag.value.size(&tcx).truncate(min);

let max = dataful_discriminant_range.end();
let max = tag.value.size(&tcx).truncate(*max);
let max = dataful_discriminant_range.end;
let max = tag.value.size(&tcx).truncate(max);

let dataful_variant_name = def.variants[*dataful_variant].ident.as_str();

Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_codegen_ssa/src/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,15 +310,15 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {

let er = scalar.valid_range_exclusive(bx.cx());
if er.end != er.start
&& scalar.valid_range.end() >= scalar.valid_range.start()
&& scalar.valid_range.end >= scalar.valid_range.start
{
// We want `table[e as usize ± k]` to not
// have bound checks, and this is the most
// convenient place to put the `assume`s.
if *scalar.valid_range.start() > 0 {
if scalar.valid_range.start > 0 {
let enum_value_lower_bound = bx
.cx()
.const_uint_big(ll_t_in, *scalar.valid_range.start());
.const_uint_big(ll_t_in, scalar.valid_range.start);
let cmp_start = bx.icmp(
IntPredicate::IntUGE,
llval,
Expand All @@ -328,7 +328,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
}

let enum_value_upper_bound =
bx.cx().const_uint_big(ll_t_in, *scalar.valid_range.end());
bx.cx().const_uint_big(ll_t_in, scalar.valid_range.end);
let cmp_end = bx.icmp(
IntPredicate::IntULE,
llval,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_lint/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -797,7 +797,7 @@ crate fn repr_nullable_ptr<'tcx>(
// Return the nullable type this Option-like enum can be safely represented with.
let field_ty_abi = &cx.layout_of(field_ty).unwrap().abi;
if let Abi::Scalar(field_ty_scalar) = field_ty_abi {
match (field_ty_scalar.valid_range.start(), field_ty_scalar.valid_range.end()) {
match (field_ty_scalar.valid_range.start, field_ty_scalar.valid_range.end) {
(0, _) => unreachable!("Non-null optimisation extended to a non-zero value."),
(1, _) => {
return Some(get_nullable_type(cx, field_ty).unwrap());
Expand Down
39 changes: 23 additions & 16 deletions compiler/rustc_middle/src/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
let scalar_unit = |value: Primitive| {
let bits = value.size(dl).bits();
assert!(bits <= 128);
Scalar { value, valid_range: 0..=(!0 >> (128 - bits)) }
Scalar { value, valid_range: WrappingRange { start: 0, end: (!0 >> (128 - bits)) } }
};
let scalar = |value: Primitive| tcx.intern_layout(Layout::scalar(self, scalar_unit(value)));

Expand All @@ -512,11 +512,14 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
// Basic scalars.
ty::Bool => tcx.intern_layout(Layout::scalar(
self,
Scalar { value: Int(I8, false), valid_range: 0..=1 },
Scalar { value: Int(I8, false), valid_range: WrappingRange { start: 0, end: 1 } },
)),
ty::Char => tcx.intern_layout(Layout::scalar(
self,
Scalar { value: Int(I32, false), valid_range: 0..=0x10FFFF },
Scalar {
value: Int(I32, false),
valid_range: WrappingRange { start: 0, end: 0x10FFFF },
},
)),
ty::Int(ity) => scalar(Int(Integer::from_int_ty(dl, ity), true)),
ty::Uint(ity) => scalar(Int(Integer::from_uint_ty(dl, ity), false)),
Expand All @@ -526,7 +529,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
}),
ty::FnPtr(_) => {
let mut ptr = scalar_unit(Pointer);
ptr.valid_range = 1..=*ptr.valid_range.end();
ptr.valid_range = ptr.valid_range.with_start(1);
tcx.intern_layout(Layout::scalar(self, ptr))
}

Expand All @@ -544,7 +547,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
ty::Ref(_, pointee, _) | ty::RawPtr(ty::TypeAndMut { ty: pointee, .. }) => {
let mut data_ptr = scalar_unit(Pointer);
if !ty.is_unsafe_ptr() {
data_ptr.valid_range = 1..=*data_ptr.valid_range.end();
data_ptr.valid_range = data_ptr.valid_range.with_start(1);
}

let pointee = tcx.normalize_erasing_regions(param_env, pointee);
Expand All @@ -560,7 +563,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
ty::Slice(_) | ty::Str => scalar_unit(Int(dl.ptr_sized_integer(), false)),
ty::Dynamic(..) => {
let mut vtable = scalar_unit(Pointer);
vtable.valid_range = 1..=*vtable.valid_range.end();
vtable.valid_range = vtable.valid_range.with_start(1);
vtable
}
_ => return Err(LayoutError::Unknown(unsized_part)),
Expand Down Expand Up @@ -933,14 +936,14 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
if let Bound::Included(start) = start {
// FIXME(eddyb) this might be incorrect - it doesn't
// account for wrap-around (end < start) ranges.
assert!(*scalar.valid_range.start() <= start);
scalar.valid_range = start..=*scalar.valid_range.end();
assert!(scalar.valid_range.start <= start);
scalar.valid_range.start = start;
}
if let Bound::Included(end) = end {
// FIXME(eddyb) this might be incorrect - it doesn't
// account for wrap-around (end < start) ranges.
assert!(*scalar.valid_range.end() >= end);
scalar.valid_range = *scalar.valid_range.start()..=end;
assert!(scalar.valid_range.end >= end);
scalar.valid_range.end = end;
}

// Update `largest_niche` if we have introduced a larger niche.
Expand Down Expand Up @@ -1256,7 +1259,10 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
let tag_mask = !0u128 >> (128 - ity.size().bits());
let tag = Scalar {
value: Int(ity, signed),
valid_range: (min as u128 & tag_mask)..=(max as u128 & tag_mask),
valid_range: WrappingRange {
start: (min as u128 & tag_mask),
end: (max as u128 & tag_mask),
},
};
let mut abi = Abi::Aggregate { sized: true };
if tag.value.size(dl) == size {
Expand Down Expand Up @@ -1535,7 +1541,10 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
let max_discr = (info.variant_fields.len() - 1) as u128;
let discr_int = Integer::fit_unsigned(max_discr);
let discr_int_ty = discr_int.to_ty(tcx, false);
let tag = Scalar { value: Primitive::Int(discr_int, false), valid_range: 0..=max_discr };
let tag = Scalar {
value: Primitive::Int(discr_int, false),
valid_range: WrappingRange { start: 0, end: max_discr },
};
let tag_layout = self.tcx.intern_layout(Layout::scalar(self, tag.clone()));
let tag_layout = TyAndLayout { ty: discr_int_ty, layout: tag_layout };

Expand Down Expand Up @@ -2846,10 +2855,8 @@ where
return;
}

if scalar.valid_range.start() < scalar.valid_range.end() {
if *scalar.valid_range.start() > 0 {
attrs.set(ArgAttribute::NonNull);
}
if !scalar.valid_range.contains_zero() {
attrs.set(ArgAttribute::NonNull);
}

if let Some(pointee) = layout.pointee_info_at(cx, offset) {
Expand Down
27 changes: 8 additions & 19 deletions compiler/rustc_mir/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,16 @@
use std::convert::TryFrom;
use std::fmt::Write;
use std::num::NonZeroUsize;
use std::ops::RangeInclusive;

use rustc_data_structures::fx::FxHashSet;
use rustc_hir as hir;
use rustc_middle::mir::interpret::InterpError;
use rustc_middle::ty;
use rustc_middle::ty::layout::TyAndLayout;
use rustc_span::symbol::{sym, Symbol};
use rustc_target::abi::{Abi, LayoutOf, Scalar as ScalarAbi, Size, VariantIdx, Variants};
use rustc_target::abi::{
Abi, LayoutOf, Scalar as ScalarAbi, Size, VariantIdx, Variants, WrappingRange,
};

use std::hash::Hash;

Expand Down Expand Up @@ -181,22 +182,10 @@ fn write_path(out: &mut String, path: &[PathElem]) {
}
}

// Test if a range that wraps at overflow contains `test`
fn wrapping_range_contains(r: &RangeInclusive<u128>, test: u128) -> bool {
let (lo, hi) = r.clone().into_inner();
if lo > hi {
// Wrapped
(..=hi).contains(&test) || (lo..).contains(&test)
} else {
// Normal
r.contains(&test)
}
}

// Formats such that a sentence like "expected something {}" to mean
// "expected something <in the given range>" makes sense.
fn wrapping_range_format(r: &RangeInclusive<u128>, max_hi: u128) -> String {
let (lo, hi) = r.clone().into_inner();
fn wrapping_range_format(r: WrappingRange, max_hi: u128) -> String {
let WrappingRange { start: lo, end: hi } = r;
assert!(hi <= max_hi);
if lo > hi {
format!("less or equal to {}, or greater or equal to {}", hi, lo)
Expand Down Expand Up @@ -634,8 +623,8 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
scalar_layout: &ScalarAbi,
) -> InterpResult<'tcx> {
let value = self.read_scalar(op)?;
let valid_range = &scalar_layout.valid_range;
let (lo, hi) = valid_range.clone().into_inner();
let valid_range = scalar_layout.valid_range.clone();
let WrappingRange { start: lo, end: hi } = valid_range;
// Determine the allowed range
// `max_hi` is as big as the size fits
let max_hi = u128::MAX >> (128 - op.layout.size.bits());
Expand Down Expand Up @@ -684,7 +673,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
Ok(int) => int.assert_bits(op.layout.size),
};
// Now compare. This is slightly subtle because this is a special "wrap-around" range.
if wrapping_range_contains(&valid_range, bits) {
if valid_range.contains(bits) {
Ok(())
} else {
throw_validation_failure!(self.path,
Expand Down
Loading