Skip to content
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

Pack u128 in the compiler to mitigate new alignment #120080

Merged
merged 3 commits into from
Jan 22, 2024
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
11 changes: 4 additions & 7 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ pub use UnsafeSource::*;
use crate::ptr::P;
use crate::token::{self, CommentKind, Delimiter};
use crate::tokenstream::{DelimSpan, LazyAttrTokenStream, TokenStream};
use rustc_data_structures::packed::Pu128;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_data_structures::sync::Lrc;
Expand Down Expand Up @@ -1833,7 +1834,7 @@ pub enum LitKind {
/// A character literal (`'a'`).
Char(char),
/// An integer literal (`1`).
Int(u128, LitIntType),
Int(Pu128, LitIntType),
/// A float literal (`1.0`, `1f64` or `1E10f64`). The pre-suffix part is
/// stored as a symbol rather than `f64` so that `LitKind` can impl `Eq`
/// and `Hash`.
Expand Down Expand Up @@ -3304,13 +3305,9 @@ mod size_asserts {
static_assert_size!(Impl, 136);
static_assert_size!(Item, 136);
static_assert_size!(ItemKind, 64);
// This can be removed after i128:128 is in the bootstrap compiler's target.
#[cfg(not(bootstrap))]
static_assert_size!(LitKind, 32);
static_assert_size!(LitKind, 24);
static_assert_size!(Local, 72);
// This can be removed after i128:128 is in the bootstrap compiler's target.
#[cfg(not(bootstrap))]
static_assert_size!(MetaItemLit, 48);
static_assert_size!(MetaItemLit, 40);
static_assert_size!(Param, 40);
static_assert_size!(Pat, 72);
static_assert_size!(Path, 24);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/util/literal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ fn integer_lit(symbol: Symbol, suffix: Option<Symbol>) -> Result<LitKind, LitErr
};

let s = &s[if base != 10 { 2 } else { 0 }..];
u128::from_str_radix(s, base).map(|i| LitKind::Int(i, ty)).map_err(|_| {
u128::from_str_radix(s, base).map(|i| LitKind::Int(i.into(), ty)).map_err(|_| {
// Small bases are lexed as if they were base 10, e.g, the string
// might be `0b10201`. This will cause the conversion above to fail,
// but these kinds of errors are already reported by the lexer.
Expand Down
10 changes: 8 additions & 2 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1899,15 +1899,21 @@ impl<'hir> LoweringContext<'_, 'hir> {
pub(super) fn expr_usize(&mut self, sp: Span, value: usize) -> hir::Expr<'hir> {
let lit = self.arena.alloc(hir::Lit {
span: sp,
node: ast::LitKind::Int(value as u128, ast::LitIntType::Unsigned(ast::UintTy::Usize)),
node: ast::LitKind::Int(
(value as u128).into(),
ast::LitIntType::Unsigned(ast::UintTy::Usize),
),
});
self.expr(sp, hir::ExprKind::Lit(lit))
}

pub(super) fn expr_u32(&mut self, sp: Span, value: u32) -> hir::Expr<'hir> {
let lit = self.arena.alloc(hir::Lit {
span: sp,
node: ast::LitKind::Int(value.into(), ast::LitIntType::Unsigned(ast::UintTy::U32)),
node: ast::LitKind::Int(
u128::from(value).into(),
ast::LitIntType::Unsigned(ast::UintTy::U32),
),
});
self.expr(sp, hir::ExprKind::Lit(lit))
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_attr/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1185,9 +1185,9 @@ fn allow_unstable<'a>(

pub fn parse_alignment(node: &ast::LitKind) -> Result<u32, &'static str> {
if let ast::LitKind::Int(literal, ast::LitIntType::Unsuffixed) = node {
if literal.is_power_of_two() {
if literal.get().is_power_of_two() {
// rustc_middle::ty::layout::Align restricts align to <= 2^29
if *literal <= 1 << 29 { Ok(*literal as u32) } else { Err("larger than 2^29") }
if *literal <= 1 << 29 { Ok(literal.get() as u32) } else { Err("larger than 2^29") }
} else {
Err("not a power of two")
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_attr/src/session_diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ impl<'a> IncorrectReprFormatGenericCause<'a> {
pub fn from_lit_kind(span: Span, kind: &ast::LitKind, name: &'a str) -> Option<Self> {
match kind {
ast::LitKind::Int(int, ast::LitIntType::Unsuffixed) => {
Some(Self::Int { span, name, int: *int })
Some(Self::Int { span, name, int: int.get() })
}
ast::LitKind::Str(symbol, _) => Some(Self::Symbol { span, name, symbol: *symbol }),
_ => None,
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_builtin_macros/src/concat_bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ fn invalid_type_err(
val,
ast::LitIntType::Unsuffixed | ast::LitIntType::Unsigned(ast::UintTy::U8),
)) => {
assert!(val > u8::MAX.into()); // must be an error
assert!(val.get() > u8::MAX.into()); // must be an error
dcx.emit_err(ConcatBytesOob { span });
}
Ok(ast::LitKind::Int(_, _)) => {
Expand Down Expand Up @@ -86,7 +86,7 @@ fn handle_array_element(
Ok(ast::LitKind::Int(
val,
ast::LitIntType::Unsuffixed | ast::LitIntType::Unsigned(ast::UintTy::U8),
)) if val <= u8::MAX.into() => Some(val as u8),
)) if val.get() <= u8::MAX.into() => Some(val.get() as u8),

Ok(ast::LitKind::Byte(val)) => Some(val),
Ok(ast::LitKind::ByteStr(..)) => {
Expand Down Expand Up @@ -148,7 +148,7 @@ pub fn expand_concat_bytes(
if let Some(elem) =
handle_array_element(cx, &mut has_errors, &mut missing_literals, expr)
{
for _ in 0..count_val {
for _ in 0..count_val.get() {
accumulator.push(elem);
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/codegen_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ fn check_link_ordinal(tcx: TyCtxt<'_>, attr: &ast::Attribute) -> Option<u16> {
// if the resulting EXE runs, as I haven't yet built the necessary DLL -- see earlier comment
// about LINK.EXE failing.)
if *ordinal <= u16::MAX as u128 {
Some(*ordinal as u16)
Some(ordinal.get() as u16)
} else {
let msg = format!("ordinal value in `link_ordinal` is too large: `{}`", &ordinal);
tcx.dcx()
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_data_structures/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ pub mod aligned;
pub mod frozen;
mod hashes;
pub mod owned_slice;
pub mod packed;
pub mod sso;
pub mod steal;
pub mod tagged_ptr;
Expand Down
71 changes: 71 additions & 0 deletions compiler/rustc_data_structures/src/packed.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
use crate::stable_hasher::{HashStable, StableHasher};
use rustc_serialize::{Decodable, Decoder, Encodable, Encoder};
use std::cmp::Ordering;
use std::fmt;

#[repr(packed(8))]
#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)]
pub struct Pu128(pub u128);
Copy link
Member

Choose a reason for hiding this comment

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

Is there any reason this needs to be 8-aligned? Is there anything to be gained from reducing alignment further?

Copy link
Member Author

Choose a reason for hiding this comment

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

I just chose that to match its former alignment, which should still be a nice "native" alignment (on 64-bit hosts). We could try smaller, but in a quick local check, even a full #[repr(packed)] didn't change these static_assert_sizes any further. Generally, I suspect there's almost always going to be a usize or pointer nearby keeping overall struct alignment up.

Maybe we should make it match target_pointer_width? But we don't test performance on other targets...


impl Pu128 {
#[inline]
pub fn get(self) -> u128 {
self.0
}
}

impl From<u128> for Pu128 {
#[inline]
fn from(value: u128) -> Self {
Self(value)
}
}

impl PartialEq<u128> for Pu128 {
#[inline]
fn eq(&self, other: &u128) -> bool {
({ self.0 }) == *other
}
}

impl PartialOrd<u128> for Pu128 {
#[inline]
fn partial_cmp(&self, other: &u128) -> Option<Ordering> {
{ self.0 }.partial_cmp(other)
}
}

impl fmt::Display for Pu128 {
#[inline]
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
{ self.0 }.fmt(f)
}
}

impl fmt::UpperHex for Pu128 {
#[inline]
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
{ self.0 }.fmt(f)
}
}

impl<CTX> HashStable<CTX> for Pu128 {
#[inline]
fn hash_stable(&self, ctx: &mut CTX, hasher: &mut StableHasher) {
{ self.0 }.hash_stable(ctx, hasher)
}
}

impl<S: Encoder> Encodable<S> for Pu128 {
#[inline]
fn encode(&self, s: &mut S) {
{ self.0 }.encode(s);
}
}

impl<D: Decoder> Decodable<D> for Pu128 {
#[inline]
fn decode(d: &mut D) -> Self {
Self(u128::decode(d))
}
}
2 changes: 1 addition & 1 deletion compiler/rustc_expand/src/mbe/metavar_expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ fn parse_depth<'sess>(
};
if let Ok(lit_kind) = LitKind::from_token_lit(*lit)
&& let LitKind::Int(n_u128, LitIntType::Unsuffixed) = lit_kind
&& let Ok(n_usize) = usize::try_from(n_u128)
&& let Ok(n_usize) = usize::try_from(n_u128.get())
{
Ok(n_usize)
} else {
Expand Down
9 changes: 5 additions & 4 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2981,10 +2981,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// fixed expression:
if let ExprKind::Lit(lit) = idx.kind
&& let ast::LitKind::Int(i, ast::LitIntType::Unsuffixed) = lit.node
&& i < types
.len()
.try_into()
.expect("expected tuple index to be < usize length")
&& i.get()
< types
.len()
.try_into()
.expect("expected tuple index to be < usize length")
{
err.span_suggestion(
brackets_span,
Expand Down
8 changes: 5 additions & 3 deletions compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use crate::ty::TypeAndMut;
use core::cmp::min;
use core::iter;
use rustc_ast::util::parser::{ExprPrecedence, PREC_POSTFIX};
use rustc_data_structures::packed::Pu128;
use rustc_errors::{Applicability, Diagnostic, MultiSpan};
use rustc_hir as hir;
use rustc_hir::def::Res;
Expand Down Expand Up @@ -1409,8 +1410,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
let (_, suffix) = snippet.split_at(snippet.len() - 3);
let value = match suffix {
"f32" => (lit - 0xf32) / (16 * 16 * 16),
"f64" => (lit - 0xf64) / (16 * 16 * 16),
"f32" => (lit.get() - 0xf32) / (16 * 16 * 16),
"f64" => (lit.get() - 0xf64) / (16 * 16 * 16),
_ => return false,
};
err.span_suggestions(
Expand Down Expand Up @@ -1440,7 +1441,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
};

// Provided expression needs to be a literal `0`.
let ExprKind::Lit(Spanned { node: rustc_ast::LitKind::Int(0, _), span }) = expr.kind else {
let ExprKind::Lit(Spanned { node: rustc_ast::LitKind::Int(Pu128(0), _), span }) = expr.kind
else {
return false;
};

Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_hir_typeck/src/op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use super::method::MethodCallee;
use super::{has_expected_num_generic_args, FnCtxt};
use crate::Expectation;
use rustc_ast as ast;
use rustc_data_structures::packed::Pu128;
use rustc_errors::{struct_span_code_err, Applicability, Diagnostic, DiagnosticBuilder};
use rustc_hir as hir;
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
Expand Down Expand Up @@ -834,7 +835,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
hir::Expr {
kind:
hir::ExprKind::Lit(Spanned {
node: ast::LitKind::Int(1, _),
node: ast::LitKind::Int(Pu128(1), _),
..
}),
..
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_lint/src/invalid_from_utf8.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ impl<'tcx> LateLintPass<'tcx> for InvalidFromUtf8 {
.map(|e| match &e.kind {
ExprKind::Lit(Spanned { node: lit, .. }) => match lit {
LitKind::Byte(b) => Some(*b),
LitKind::Int(b, _) => Some(*b as u8),
LitKind::Int(b, _) => Some(b.get() as u8),
_ => None,
},
_ => None,
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_lint/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ fn lint_uint_literal<'tcx>(
let lit_val: u128 = match lit.node {
// _v is u8, within range by definition
ast::LitKind::Byte(_v) => return,
ast::LitKind::Int(v, _) => v,
ast::LitKind::Int(v, _) => v.get(),
_ => bug!(),
};
if lit_val < min || lit_val > max {
Expand Down Expand Up @@ -555,7 +555,7 @@ fn lint_literal<'tcx>(
ty::Int(t) => {
match lit.node {
ast::LitKind::Int(v, ast::LitIntType::Signed(_) | ast::LitIntType::Unsuffixed) => {
lint_int_literal(cx, type_limits, e, lit, t, v)
lint_int_literal(cx, type_limits, e, lit, t, v.get())
}
_ => bug!(),
};
Expand Down Expand Up @@ -842,7 +842,7 @@ impl<'tcx> LateLintPass<'tcx> for TypeLimits {
ast::LitKind::Int(
v,
ast::LitIntType::Signed(_) | ast::LitIntType::Unsuffixed,
) => v as i128,
) => v.get() as i128,
_ => return true,
},
_ => bug!(),
Expand All @@ -853,7 +853,7 @@ impl<'tcx> LateLintPass<'tcx> for TypeLimits {
let (min, max): (u128, u128) = uint_ty_range(uint_ty);
let lit_val: u128 = match lit.kind {
hir::ExprKind::Lit(li) => match li.node {
ast::LitKind::Int(v, _) => v,
ast::LitKind::Int(v, _) => v.get(),
_ => return true,
},
_ => bug!(),
Expand Down
12 changes: 3 additions & 9 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1672,19 +1672,13 @@ mod size_asserts {
use super::*;
use rustc_data_structures::static_assert_size;
// tidy-alphabetical-start
// This can be removed after i128:128 is in the bootstrap compiler's target.
#[cfg(not(bootstrap))]
static_assert_size!(BasicBlockData<'_>, 144);
static_assert_size!(BasicBlockData<'_>, 136);
static_assert_size!(LocalDecl<'_>, 40);
static_assert_size!(SourceScopeData<'_>, 72);
static_assert_size!(Statement<'_>, 32);
static_assert_size!(StatementKind<'_>, 16);
// This can be removed after i128:128 is in the bootstrap compiler's target.
#[cfg(not(bootstrap))]
static_assert_size!(Terminator<'_>, 112);
// This can be removed after i128:128 is in the bootstrap compiler's target.
#[cfg(not(bootstrap))]
static_assert_size!(TerminatorKind<'_>, 96);
static_assert_size!(Terminator<'_>, 104);
static_assert_size!(TerminatorKind<'_>, 88);
static_assert_size!(VarDebugInfo<'_>, 88);
// tidy-alphabetical-end
}
3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/mir/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use crate::ty::{self, List, Ty};
use crate::ty::{Region, UserTypeAnnotationIndex};

use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece};
use rustc_data_structures::packed::Pu128;
use rustc_hir::def_id::DefId;
use rustc_hir::{self, CoroutineKind};
use rustc_index::IndexVec;
Expand Down Expand Up @@ -829,7 +830,7 @@ impl TerminatorKind<'_> {
pub struct SwitchTargets {
/// Possible values. The locations to branch to in each case
/// are found in the corresponding indices from the `targets` vector.
pub(super) values: SmallVec<[u128; 1]>,
pub(super) values: SmallVec<[Pu128; 1]>,

/// Possible branch sites. The last element of this vector is used
/// for the otherwise branch, so targets.len() == values.len() + 1
Expand Down
Loading
Loading