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

[WIP] Implement unchecked_disjoint_bitor #124601

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_cranelift/src/codegen_i128.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub(crate) fn maybe_codegen<'tcx>(
let is_signed = type_sign(lhs.layout().ty);

match bin_op {
BinOp::BitAnd | BinOp::BitOr | BinOp::BitXor => None,
BinOp::BitAnd | BinOp::BitOr | BinOp::BitXor | BinOp::BitOrDisjoint => None,
BinOp::Add | BinOp::AddUnchecked | BinOp::Sub | BinOp::SubUnchecked => None,
BinOp::Mul | BinOp::MulUnchecked => {
let args = [lhs.load_scalar(fx), rhs.load_scalar(fx)];
Expand Down Expand Up @@ -90,7 +90,7 @@ pub(crate) fn maybe_codegen_checked<'tcx>(
let is_signed = type_sign(lhs.layout().ty);

match bin_op {
BinOp::BitAnd | BinOp::BitOr | BinOp::BitXor => unreachable!(),
BinOp::BitAnd | BinOp::BitOr | BinOp::BitXor | BinOp::BitOrDisjoint => unreachable!(),
BinOp::Mul if is_signed => {
let out_ty = Ty::new_tup(fx.tcx, &[lhs.layout().ty, fx.tcx.types.bool]);
let oflow = CPlace::new_stack_slot(fx, fx.layout_of(fx.tcx.types.i32));
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_codegen_cranelift/src/num.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ pub(crate) fn codegen_int_binop<'tcx>(

let b = fx.bcx.ins();
// FIXME trap on overflow for the Unchecked versions
// FIXME trap on non-disjoint for BitOrDisjoint
let val = match bin_op {
BinOp::Add | BinOp::AddUnchecked => b.iadd(lhs, rhs),
BinOp::Sub | BinOp::SubUnchecked => b.isub(lhs, rhs),
Expand All @@ -169,7 +170,7 @@ pub(crate) fn codegen_int_binop<'tcx>(
}
BinOp::BitXor => b.bxor(lhs, rhs),
BinOp::BitAnd => b.band(lhs, rhs),
BinOp::BitOr => b.bor(lhs, rhs),
BinOp::BitOr | BinOp::BitOrDisjoint => b.bor(lhs, rhs),
BinOp::Shl | BinOp::ShlUnchecked => b.ishl(lhs, rhs),
BinOp::Shr | BinOp::ShrUnchecked => {
if signed {
Expand Down
13 changes: 7 additions & 6 deletions compiler/rustc_codegen_gcc/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use rustc_middle::ty::layout::{
FnAbiError, FnAbiOfHelpers, FnAbiRequest, HasParamEnv, HasTyCtxt, LayoutError, LayoutOfHelpers,
TyAndLayout,
};
use rustc_middle::ty::{ParamEnv, Ty, TyCtxt, Instance};
use rustc_middle::ty::{Instance, ParamEnv, Ty, TyCtxt};
use rustc_span::def_id::DefId;
use rustc_span::Span;
use rustc_target::abi::{
Expand Down Expand Up @@ -792,6 +792,11 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {
self.cx.gcc_or(a, b, self.location)
}

fn disjoint_or(&mut self, a: RValue<'gcc>, b: RValue<'gcc>) -> RValue<'gcc> {
// TODO: investigate gcc equivalent of this operation
self.cx.gcc_or(a, b, self.location)
}

fn xor(&mut self, a: RValue<'gcc>, b: RValue<'gcc>) -> RValue<'gcc> {
set_rvalue_location(self, self.gcc_xor(a, b))
}
Expand Down Expand Up @@ -903,11 +908,7 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {
// TODO(antoyo): It might be better to return a LValue, but fixing the rustc API is non-trivial.
self.stack_var_count.set(self.stack_var_count.get() + 1);
self.current_func()
.new_local(
self.location,
ty,
&format!("stack_var_{}", self.stack_var_count.get()),
)
.new_local(self.location, ty, &format!("stack_var_{}", self.stack_var_count.get()))
.get_address(self.location)
}

Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_codegen_llvm/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,14 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
unchecked_umul(x, y) => LLVMBuildNUWMul,
}

fn disjoint_or(&mut self, a: &'ll Value, b: &'ll Value) -> &'ll Value {
unsafe {
let or = llvm::LLVMBuildOr(self.llbuilder, a, b, UNNAMED);
llvm::LLVMSetIsDisjoint(or, True);
or
}
}

fn fadd_fast(&mut self, lhs: &'ll Value, rhs: &'ll Value) -> &'ll Value {
unsafe {
let instr = llvm::LLVMBuildFAdd(self.llbuilder, lhs, rhs, UNNAMED);
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_llvm/src/llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1273,6 +1273,7 @@ extern "C" {
pub fn LLVMBuildNeg<'a>(B: &Builder<'a>, V: &'a Value, Name: *const c_char) -> &'a Value;
pub fn LLVMBuildFNeg<'a>(B: &Builder<'a>, V: &'a Value, Name: *const c_char) -> &'a Value;
pub fn LLVMBuildNot<'a>(B: &Builder<'a>, V: &'a Value, Name: *const c_char) -> &'a Value;
pub fn LLVMSetIsDisjoint(Inst: &Value, IsDisjoint: Bool);

// Memory
pub fn LLVMBuildAlloca<'a>(B: &Builder<'a>, Ty: &'a Type, Name: *const c_char) -> &'a Value;
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/src/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -867,6 +867,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
mir::BinOp::BitOr => bx.or(lhs, rhs),
mir::BinOp::BitAnd => bx.and(lhs, rhs),
mir::BinOp::BitXor => bx.xor(lhs, rhs),
mir::BinOp::BitOrDisjoint => bx.disjoint_or(lhs, rhs),
mir::BinOp::Offset => {
let pointee_type = input_ty
.builtin_deref(true)
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/src/traits/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ pub trait BuilderMethods<'a, 'tcx>:
fn and(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value;
fn or(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value;
fn xor(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value;
fn disjoint_or(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value;
fn neg(&mut self, v: Self::Value) -> Self::Value;
fn fneg(&mut self, v: Self::Value) -> Self::Value;
fn not(&mut self, v: Self::Value) -> Self::Value;
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_const_eval/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,9 @@ const_eval_overflow =
const_eval_overflow_shift =
overflowing shift by {$val} in `{$name}`

const_eval_overlapping_disjoint_bit_or =
disjoint bitor executed on overlapping operands (`lhs & rhs != 0`)

const_eval_panic =
the evaluated program panicked at '{$msg}', {$file}:{$line}:{$col}

Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_const_eval/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,7 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
RemainderByZero => const_eval_remainder_by_zero,
DivisionOverflow => const_eval_division_overflow,
RemainderOverflow => const_eval_remainder_overflow,
OverlappingDisjointBitOr => const_eval_overlapping_disjoint_bit_or,
PointerArithOverflow => const_eval_pointer_arithmetic_overflow,
InvalidMeta(InvalidMetaKind::SliceTooBig) => const_eval_invalid_meta_slice,
InvalidMeta(InvalidMetaKind::TooBig) => const_eval_invalid_meta,
Expand Down Expand Up @@ -531,6 +532,7 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
| RemainderByZero
| DivisionOverflow
| RemainderOverflow
| OverlappingDisjointBitOr
| PointerArithOverflow
| InvalidMeta(InvalidMetaKind::SliceTooBig)
| InvalidMeta(InvalidMetaKind::TooBig)
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_const_eval/src/interpret/operator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
BitAnd => ImmTy::from_uint(l & r, left.layout),
BitXor => ImmTy::from_uint(l ^ r, left.layout),

BitOrDisjoint => {
if l & r != 0 {
throw_ub!(OverlappingDisjointBitOr)
}
ImmTy::from_uint(l | r, left.layout)
}

Add | AddUnchecked | Sub | SubUnchecked | Mul | MulUnchecked | Rem | Div => {
assert!(!left.layout.abi.is_signed());
let op: fn(u128, u128) -> (u128, bool) = match bin_op {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/transform/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1046,7 +1046,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
)
}
}
BitAnd | BitOr | BitXor => {
BitAnd | BitOr | BitXor | BitOrDisjoint => {
for x in [a, b] {
check_kinds!(
x,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub fn binop_left_homogeneous(op: mir::BinOp) -> bool {
use rustc_middle::mir::BinOp::*;
match op {
Add | AddUnchecked | Sub | SubUnchecked | Mul | MulUnchecked | Div | Rem | BitXor
| BitAnd | BitOr | Offset | Shl | ShlUnchecked | Shr | ShrUnchecked => true,
| BitAnd | BitOr | BitOrDisjoint | Offset | Shl | ShlUnchecked | Shr | ShrUnchecked => true,
Eq | Ne | Lt | Le | Gt | Ge | Cmp => false,
}
}
Expand All @@ -30,7 +30,7 @@ pub fn binop_right_homogeneous(op: mir::BinOp) -> bool {
use rustc_middle::mir::BinOp::*;
match op {
Add | AddUnchecked | Sub | SubUnchecked | Mul | MulUnchecked | Div | Rem | BitXor
| BitAnd | BitOr | Eq | Ne | Lt | Le | Gt | Ge | Cmp => true,
| BitAnd | BitOr | BitOrDisjoint | Eq | Ne | Lt | Le | Gt | Ge | Cmp => true,
Offset | Shl | ShlUnchecked | Shr | ShrUnchecked => false,
}
}
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ pub fn check_intrinsic_type(
}
sym::unchecked_shl | sym::unchecked_shr => (2, 0, vec![param(0), param(1)], param(0)),
sym::rotate_left | sym::rotate_right => (1, 0, vec![param(0), tcx.types.u32], param(0)),
sym::unchecked_add | sym::unchecked_sub | sym::unchecked_mul => {
sym::unchecked_add | sym::unchecked_sub | sym::unchecked_mul | sym::disjoint_bitor => {
(1, 0, vec![param(0), param(0)], param(0))
}
sym::wrapping_add | sym::wrapping_sub | sym::wrapping_mul => {
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_middle/src/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,8 @@ pub enum UndefinedBehaviorInfo<'tcx> {
DivisionOverflow,
/// Signed remainder overflowed (INT_MIN % -1).
RemainderOverflow,
/// Disjoint bitor operation executed on overlapping bitvectors (lhs & rhs != 0).
OverlappingDisjointBitOr,
/// Overflowing inbounds pointer arithmetic.
PointerArithOverflow,
/// Invalid metadata in a wide pointer
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_middle/src/mir/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1478,6 +1478,9 @@ pub enum BinOp {
BitAnd,
/// The `|` operator (bitwise or)
BitOr,
/// Equivalent to all of `BitOr`, `BitXor`, and `Add`. UB if any bit is set
/// in both operands; that is, if `lhs & rhs != 0`.
BitOrDisjoint,
/// The `<<` operator (shift left)
///
/// The offset is truncated to the size of the first operand and made unsigned before shifting.
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_middle/src/mir/tcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,8 @@ impl<'tcx> BinOp {
| &BinOp::Rem
| &BinOp::BitXor
| &BinOp::BitAnd
| &BinOp::BitOr => {
| &BinOp::BitOr
| &BinOp::BitOrDisjoint => {
// these should be integers or floats of the same size.
assert_eq!(lhs_ty, rhs_ty);
lhs_ty
Expand Down Expand Up @@ -322,6 +323,7 @@ impl BinOp {
| BinOp::AddUnchecked
| BinOp::SubUnchecked
| BinOp::MulUnchecked
| BinOp::BitOrDisjoint
| BinOp::ShlUnchecked
| BinOp::ShrUnchecked
| BinOp::Offset => {
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_mir_transform/src/lower_intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics {
| sym::unchecked_div
| sym::unchecked_rem
| sym::unchecked_shl
| sym::unchecked_shr => {
| sym::unchecked_shr
| sym::disjoint_bitor => {
let target = target.unwrap();
let lhs;
let rhs;
Expand All @@ -118,6 +119,7 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics {
sym::unchecked_rem => BinOp::Rem,
sym::unchecked_shl => BinOp::ShlUnchecked,
sym::unchecked_shr => BinOp::ShrUnchecked,
sym::disjoint_bitor => BinOp::BitOrDisjoint,
_ => bug!("unexpected intrinsic"),
};
block.statements.push(Statement {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_transform/src/promote_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,7 @@ impl<'tcx> Validator<'_, 'tcx> {
| BinOp::BitXor
| BinOp::BitAnd
| BinOp::BitOr
| BinOp::BitOrDisjoint
| BinOp::Shl
| BinOp::ShlUnchecked
| BinOp::Shr
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_smir/src/rustc_internal/internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,7 @@ impl RustcInternal for BinOp {
BinOp::BitXor => rustc_middle::mir::BinOp::BitXor,
BinOp::BitAnd => rustc_middle::mir::BinOp::BitAnd,
BinOp::BitOr => rustc_middle::mir::BinOp::BitOr,
BinOp::BitOrDisjoint => rustc_middle::mir::BinOp::BitOrDisjoint,
BinOp::Shl => rustc_middle::mir::BinOp::Shl,
BinOp::ShlUnchecked => rustc_middle::mir::BinOp::ShlUnchecked,
BinOp::Shr => rustc_middle::mir::BinOp::Shr,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_smir/src/rustc_smir/convert/mir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,7 @@ impl<'tcx> Stable<'tcx> for mir::BinOp {
BinOp::BitXor => stable_mir::mir::BinOp::BitXor,
BinOp::BitAnd => stable_mir::mir::BinOp::BitAnd,
BinOp::BitOr => stable_mir::mir::BinOp::BitOr,
BinOp::BitOrDisjoint => stable_mir::mir::BinOp::BitOrDisjoint,
BinOp::Shl => stable_mir::mir::BinOp::Shl,
BinOp::ShlUnchecked => stable_mir::mir::BinOp::ShlUnchecked,
BinOp::Shr => stable_mir::mir::BinOp::Shr,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,7 @@ symbols! {
discriminant_kind,
discriminant_type,
discriminant_value,
disjoint_bitor,
dispatch_from_dyn,
div,
div_assign,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_ty_utils/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ fn check_binop(op: mir::BinOp) -> bool {
use mir::BinOp::*;
match op {
Add | AddUnchecked | Sub | SubUnchecked | Mul | MulUnchecked | Div | Rem | BitXor
| BitAnd | BitOr | Shl | ShlUnchecked | Shr | ShrUnchecked | Eq | Lt | Le | Ne | Ge
| Gt | Cmp => true,
| BitAnd | BitOr | BitOrDisjoint | Shl | ShlUnchecked | Shr | ShrUnchecked | Eq | Lt
| Le | Ne | Ge | Gt | Cmp => true,
Offset => false,
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/stable_mir/src/mir/body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ pub enum BinOp {
BitXor,
BitAnd,
BitOr,
BitOrDisjoint,
Shl,
ShlUnchecked,
Shr,
Expand Down
13 changes: 13 additions & 0 deletions library/core/src/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2311,6 +2311,19 @@ extern "rust-intrinsic" {
#[rustc_nounwind]
pub fn unchecked_mul<T: Copy>(x: T, y: T) -> T;

/// Performs an unchecked disjoint bitor operation. Equivalent to normal
/// bitor and bitxor operations, triggering undefined behavior if their
/// results differ.
///
/// This intrinsic does not have a stable counterpart.
// /// The stable counterpart of this intrinsic is `unchecked_disjoint_bitor`
// /// on the various integer types, such as [`u16::unchecked_disjoint_bitor`]
// /// and [`i64::unchecked_disjoint_bitor`].
#[cfg(not(bootstrap))]
#[rustc_const_unstable(feature = "disjoint_bitor", issue = "none")]
#[rustc_nounwind]
pub fn disjoint_bitor<T: Copy>(x: T, y: T) -> T;
Copy link
Member

Choose a reason for hiding this comment

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

I think, as a good way to split up the PR, this PR should just add this as an intrinsic with fallback.

You can see

pub const unsafe fn assume(b: bool) {
if !b {
// SAFETY: the caller must guarantee the argument is never `false`
unsafe { unreachable() }
}
}
as an example of that. That will let you remove all the codegen changes, and most of the compiler changes too -- it'll only need just enough compiler changes to pass the intrinsic typechecking part. And we can then approve that fairly simply, since it doesn't need to worry about LLVM version checks and such.

Then a follow-up PR can change just the llvm backend to do something different, without needing to update cg_gcc nor cg_clif nor smir nor ...

This this would be something like

Suggested change
pub fn disjoint_bitor<T: Copy>(x: T, y: T) -> T;
// The `assume` in the body is enough for MIRI to see any UB from this.
#[cfg_attr(not(bootstrap), miri::intrinsic_fallback_checks_ub)]
pub unsafe fn disjoint_bitor<T: Copy + Default + Eq>(x: T, y: T) -> T
where
T: std::ops::BitAnd<Output = T> + std::ops::BitOr<Output = T>,
{
assume(x & y == T::default());
x | y
}


/// Performs rotate left.
///
/// Note that, unlike most intrinsics, this is safe to call;
Expand Down