bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

De-LLVM the unchecked shifts [MCP#693]

Browse source 
This is just one part of the MCP, but it's the one that IMHO removes the most noise from the standard library code.

Seems net simpler this way, since MIR already supported heterogeneous shifts anyway, and thus it's not more work for backends than before.
This commit is contained in:
Scott McMurray 2024-03-30 00:36:45 -07:00
parent 69fa40cb48
commit 0601f0c66d
32 changed files with 405 additions and 576 deletions
Download patch file Download diff file Expand all files Collapse all files

32
compiler/rustc_codegen_ssa/src/base.rs
View file

@ -5,7 +5,7 @@ use crate::back::write::{
compute_per_cgu_lto_type, start_async_codegen, submit_codegened_module_to_llvm,
submit_post_lto_module_to_llvm, submit_pre_lto_module_to_llvm, ComputedLtoType, OngoingCodegen,
};
use crate::common::{IntPredicate, RealPredicate, TypeKind};
use crate::common::{self, IntPredicate, RealPredicate, TypeKind};
use crate::errors;
use crate::meth;
use crate::mir;
@ -33,7 +33,7 @@ use rustc_middle::mir::mono::{CodegenUnit, CodegenUnitNameBuilder, MonoItem};
use rustc_middle::query::Providers;
use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf, TyAndLayout};
use rustc_middle::ty::{self, Instance, Ty, TyCtxt};
use rustc_session::config::{self, CrateType, EntryFnType, OutputType};
use rustc_session::config::{self, CrateType, EntryFnType, OptLevel, OutputType};
use rustc_session::Session;
use rustc_span::symbol::sym;
use rustc_span::Symbol;
@ -300,14 +300,32 @@ pub fn coerce_unsized_into<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
}
}
pub fn cast_shift_expr_rhs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
/// Shifts in MIR are all allowed to have mismatched LHS & RHS types.
///
/// This does all the appropriate conversions needed to pass it to the builder's
/// shift methods, which are UB for out-of-range shifts.
///
/// If `is_unchecked` is false, this masks the RHS to ensure it stays in-bounds.
/// For 32- and 64-bit types, this matches the semantics
/// of Java. (See related discussion on #1877 and #10183.)
///
/// If `is_unchecked` is true, this does no masking, and adds sufficient `assume`
/// calls or operation flags to preserve as much freedom to optimize as possible.
pub fn build_shift_expr_rhs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
bx: &mut Bx,
lhs: Bx::Value,
rhs: Bx::Value,
mut rhs: Bx::Value,
is_unchecked: bool,
) -> Bx::Value {
// Shifts may have any size int on the rhs
let mut rhs_llty = bx.cx().val_ty(rhs);
let mut lhs_llty = bx.cx().val_ty(lhs);
let mask = common::shift_mask_val(bx, lhs_llty, rhs_llty, false);
if !is_unchecked {
rhs = bx.and(rhs, mask);
}
if bx.cx().type_kind(rhs_llty) == TypeKind::Vector {
rhs_llty = bx.cx().element_type(rhs_llty)
}
@ -317,6 +335,12 @@ pub fn cast_shift_expr_rhs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
let rhs_sz = bx.cx().int_width(rhs_llty);
let lhs_sz = bx.cx().int_width(lhs_llty);
if lhs_sz < rhs_sz {
if is_unchecked && bx.sess().opts.optimize != OptLevel::No {
// FIXME: Use `trunc nuw` once that's available
let inrange = bx.icmp(IntPredicate::IntULE, rhs, mask);
bx.assume(inrange);
}
bx.trunc(rhs, lhs_llty)
} else if lhs_sz > rhs_sz {
// We zero-extend even if the RHS is signed. So e.g. `(x: i32) << -1i8` will zero-extend the

41
compiler/rustc_codegen_ssa/src/common.rs
View file

@ -3,10 +3,9 @@
use rustc_hir::LangItem;
use rustc_middle::mir;
use rustc_middle::ty::Instance;
use rustc_middle::ty::{self, layout::TyAndLayout, Ty, TyCtxt};
use rustc_middle::ty::{self, layout::TyAndLayout, TyCtxt};
use rustc_span::Span;
use crate::base;
use crate::traits::*;
#[derive(Copy, Clone)]
@ -128,44 +127,6 @@ pub fn build_langcall<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
(bx.fn_abi_of_instance(instance, ty::List::empty()), bx.get_fn_addr(instance), instance)
}
// To avoid UB from LLVM, these two functions mask RHS with an
// appropriate mask unconditionally (i.e., the fallback behavior for
// all shifts). For 32- and 64-bit types, this matches the semantics
// of Java. (See related discussion on #1877 and #10183.)
pub fn build_masked_lshift<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
bx: &mut Bx,
lhs: Bx::Value,
rhs: Bx::Value,
) -> Bx::Value {
let rhs = base::cast_shift_expr_rhs(bx, lhs, rhs);
// #1877, #10183: Ensure that input is always valid
let rhs = shift_mask_rhs(bx, rhs);
bx.shl(lhs, rhs)
}
pub fn build_masked_rshift<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
bx: &mut Bx,
lhs_t: Ty<'tcx>,
lhs: Bx::Value,
rhs: Bx::Value,
) -> Bx::Value {
let rhs = base::cast_shift_expr_rhs(bx, lhs, rhs);
// #1877, #10183: Ensure that input is always valid
let rhs = shift_mask_rhs(bx, rhs);
let is_signed = lhs_t.is_signed();
if is_signed { bx.ashr(lhs, rhs) } else { bx.lshr(lhs, rhs) }
}
fn shift_mask_rhs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
bx: &mut Bx,
rhs: Bx::Value,
) -> Bx::Value {
let rhs_llty = bx.val_ty(rhs);
let shift_val = shift_mask_val(bx, rhs_llty, rhs_llty, false);
bx.and(rhs, shift_val)
}
pub fn shift_mask_val<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
bx: &mut Bx,
llty: Bx::Type,

12
compiler/rustc_codegen_ssa/src/mir/rvalue.rs
View file

@ -3,7 +3,7 @@ use super::place::PlaceRef;
use super::{FunctionCx, LocalRef};
use crate::base;
use crate::common::{self, IntPredicate};
use crate::common::IntPredicate;
use crate::traits::*;
use crate::MemFlags;
@ -860,14 +860,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
bx.inbounds_gep(llty, lhs, &[rhs])
}
}
mir::BinOp::Shl => common::build_masked_lshift(bx, lhs, rhs),
mir::BinOp::ShlUnchecked => {
let rhs = base::cast_shift_expr_rhs(bx, lhs, rhs);
mir::BinOp::Shl | mir::BinOp::ShlUnchecked => {
let rhs = base::build_shift_expr_rhs(bx, lhs, rhs, op == mir::BinOp::ShlUnchecked);
bx.shl(lhs, rhs)
}
mir::BinOp::Shr => common::build_masked_rshift(bx, input_ty, lhs, rhs),
mir::BinOp::ShrUnchecked => {
let rhs = base::cast_shift_expr_rhs(bx, lhs, rhs);
mir::BinOp::Shr | mir::BinOp::ShrUnchecked => {
let rhs = base::build_shift_expr_rhs(bx, lhs, rhs, op == mir::BinOp::ShrUnchecked);
if is_signed { bx.ashr(lhs, rhs) } else { bx.lshr(lhs, rhs) }
}
mir::BinOp::Ne