Skip to content

Commit

Permalink
Introduce a special case in IntRange::from_const.
Browse files Browse the repository at this point in the history
The `if let Some(val) = value.try_eval_bits(...)` branch in `from_const()` is
very hot for the `unicode_normalization` benchmark.

This commit introduces a special-case alternative for scalars that avoids
`try_eval_bits()` and all the functions it calls (`Const::eval()`,
`ConstValue::try_to_bits()`, `ConstValue::try_to_scalar()`, and
`Scalar::to_bits()`), instead extracting the result immediately.

The type and value checking done by `Scalar::to_bits()` is replicated by moving
it into a new function `Scalar::check_raw()` and using that new function in the
special case.

PR rust-lang#64673 introduced some special-case handling of scalar types in
`Const::try_eval_bits()`. This handling is now moved out of that function into
the new `IntRange::integral_size_and_signed_bias` function.

This commit reduces the instruction count for
`unicode_normalization-check-clean` by about 10%.
  • Loading branch information
nnethercote committed Oct 4, 2019
1 parent a69e0e0 commit 5515a97
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 26 deletions.
11 changes: 8 additions & 3 deletions src/librustc/mir/interpret/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -343,14 +343,19 @@ impl<'tcx, Tag> Scalar<Tag> {
}
}

#[inline(always)]
pub fn check_raw(data: u128, size: u8, target_size: Size) {
assert_eq!(target_size.bytes(), size as u64);
assert_ne!(size, 0, "you should never look at the bits of a ZST");
Scalar::check_data(data, size);
}

/// Do not call this method! Use either `assert_bits` or `force_bits`.
#[inline]
pub fn to_bits(self, target_size: Size) -> InterpResult<'tcx, u128> {
match self {
Scalar::Raw { data, size } => {
assert_eq!(target_size.bytes(), size as u64);
assert_ne!(size, 0, "you should never look at the bits of a ZST");
Scalar::check_data(data, size);
Self::check_raw(data, size, target_size);
Ok(data)
}
Scalar::Ptr(_) => throw_unsup!(ReadPointerAsBytes),
Expand Down
18 changes: 2 additions & 16 deletions src/librustc/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_macros::HashStable;
use crate::ty::subst::{InternalSubsts, Subst, SubstsRef, GenericArg, GenericArgKind};
use crate::ty::{self, AdtDef, Discr, DefIdTree, TypeFlags, Ty, TyCtxt, TypeFoldable};
use crate::ty::{List, TyS, ParamEnvAnd, ParamEnv};
use crate::ty::layout::{Size, Integer, IntegerExt, VariantIdx};
use crate::ty::layout::VariantIdx;
use crate::util::captures::Captures;
use crate::mir::interpret::{Scalar, GlobalId};

Expand All @@ -24,7 +24,6 @@ use std::marker::PhantomData;
use std::ops::Range;
use rustc_target::spec::abi;
use syntax::ast::{self, Ident};
use syntax::attr::{SignedInt, UnsignedInt};
use syntax::symbol::{kw, InternedString};

use self::InferTy::*;
Expand Down Expand Up @@ -2299,20 +2298,7 @@ impl<'tcx> Const<'tcx> {
ty: Ty<'tcx>,
) -> Option<u128> {
assert_eq!(self.ty, ty);
// This is purely an optimization -- layout_of is a pretty expensive operation,
// but if we can determine the size without calling it, we don't need all that complexity
// (hashing, caching, etc.). As such, try to skip it.
let size = match ty.kind {
ty::Bool => Size::from_bytes(1),
ty::Char => Size::from_bytes(4),
ty::Int(ity) => {
Integer::from_attr(&tcx, SignedInt(ity)).size()
}
ty::Uint(uty) => {
Integer::from_attr(&tcx, UnsignedInt(uty)).size()
}
_ => tcx.layout_of(param_env.with_reveal_all().and(ty)).ok()?.size,
};
let size = tcx.layout_of(param_env.with_reveal_all().and(ty)).ok()?.size;
// if `ty` does not depend on generic parameters, use an empty param_env
self.eval(tcx, param_env).val.try_to_bits(size)
}
Expand Down
35 changes: 28 additions & 7 deletions src/librustc_mir/hair/pattern/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -842,21 +842,42 @@ impl<'tcx> IntRange<'tcx> {
}
}

#[inline]
fn integral_size_and_signed_bias(tcx: TyCtxt<'tcx>, ty: Ty<'_>) -> Option<(Size, u128)> {
match ty.kind {
ty::Char => Some((Size::from_bytes(4), 0)),
ty::Int(ity) => {
let size = Integer::from_attr(&tcx, SignedInt(ity)).size();
Some((size, 1u128 << (size.bits() as u128 - 1)))
}
ty::Uint(uty) => Some((Integer::from_attr(&tcx, UnsignedInt(uty)).size(), 0)),
_ => None,
}
}

#[inline]
fn from_const(
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
value: &Const<'tcx>,
) -> Option<IntRange<'tcx>> {
if Self::is_integral(value.ty) {
if let Some((target_size, bias)) = Self::integral_size_and_signed_bias(tcx, value.ty) {
let ty = value.ty;
if let Some(val) = value.try_eval_bits(tcx, param_env, ty) {
let bias = IntRange::signed_bias(tcx, ty);
let val = val ^ bias;
Some(IntRange { range: val..=val, ty })
let val = if let ConstValue::Scalar(Scalar::Raw { data, size }) = value.val {
// For this specific pattern we can skip a lot of effort and go
// straight to the result, after doing a bit of checking. (We
// could remove this branch and just use the next branch, which
// is more general but much slower.)
Scalar::<()>::check_raw(data, size, target_size);
data
} else if let Some(val) = value.try_eval_bits(tcx, param_env, ty) {
// This is a more general form of the previous branch.
val
} else {
None
}
return None
};
let val = val ^ bias;
Some(IntRange { range: val..=val, ty })
} else {
None
}
Expand Down

0 comments on commit 5515a97

Please sign in to comment.