Skip to content

Commit

Permalink
Auto merge of #122282 - DianQK:simplify_ub_check, r=<try>
Browse files Browse the repository at this point in the history
Eliminate `UbCheck` for non-standard libraries

Since only core and std are prebuilt crates, I think it's only necessary to keep UB checks for them.
`mir-opt` may not optimize some code due to `UbCheck`.
In more complex cases, I'm also not sure that LLVM utilizes `assume` correctly.

r? saethlin
  • Loading branch information
bors committed Mar 10, 2024
2 parents cdb775c + e6474b8 commit acc30d0
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 0 deletions.
5 changes: 5 additions & 0 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -772,6 +772,11 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
rustc_allow_incoherent_impl, AttributeType::Normal, template!(Word), ErrorFollowing, @only_local: true,
"#[rustc_allow_incoherent_impl] has to be added to all impl items of an incoherent inherent impl."
),
rustc_attr!(
rustc_ub_check, AttributeType::CrateLevel, template!(Word), ErrorFollowing,
@only_local: true,
"`#![rustc_ub_check]` is an internal feature of the standard library, marks annotated code to not have UB checks optimized.",
),
rustc_attr!(
rustc_deny_explicit_impl,
AttributeType::Normal,
Expand Down
27 changes: 27 additions & 0 deletions compiler/rustc_mir_transform/src/instsimplify.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
//! Performs various peephole optimizations.

use crate::simplify::simplify_duplicate_switch_targets;
use rustc_ast::attr;
use rustc_middle::mir::*;
use rustc_middle::ty::layout;
use rustc_middle::ty::layout::ValidityRequirement;
use rustc_middle::ty::{self, GenericArgsRef, ParamEnv, Ty, TyCtxt};
use rustc_span::sym;
use rustc_span::symbol::Symbol;
use rustc_target::abi::FieldIdx;
use rustc_target::spec::abi::Abi;
Expand All @@ -22,10 +24,17 @@ impl<'tcx> MirPass<'tcx> for InstSimplify {
local_decls: &body.local_decls,
param_env: tcx.param_env_reveal_all_normalized(body.source.def_id()),
};
// FIXME(#116171) Coverage related, also see `unreachable_prop.rs`.
let rustc_ub_check = attr::contains_name(tcx.hir().krate_attrs(), sym::rustc_ub_check)
|| tcx.sess.instrument_coverage();
let debug_assertions = tcx.sess.opts.debug_assertions;
for block in body.basic_blocks.as_mut() {
for statement in block.statements.iter_mut() {
match statement.kind {
StatementKind::Assign(box (_place, ref mut rvalue)) => {
if !rustc_ub_check {
ctx.simplify_ub_check(&statement.source_info, rvalue, debug_assertions);
}
ctx.simplify_bool_cmp(&statement.source_info, rvalue);
ctx.simplify_ref_deref(&statement.source_info, rvalue);
ctx.simplify_len(&statement.source_info, rvalue);
Expand Down Expand Up @@ -140,6 +149,24 @@ impl<'tcx> InstSimplifyContext<'tcx, '_> {
}
}

fn simplify_ub_check(
&self,
source_info: &SourceInfo,
rvalue: &mut Rvalue<'tcx>,
debug_assertions: bool,
) {
if let Rvalue::NullaryOp(ref null_op, _) = *rvalue {
match null_op {
NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(_) => {}
NullOp::UbCheck(_) => {
let const_ = Const::from_bool(self.tcx, debug_assertions);
let constant = ConstOperand { span: source_info.span, const_, user_ty: None };
*rvalue = Rvalue::Use(Operand::Constant(Box::new(constant)));
}
}
}
}

fn simplify_cast(&self, rvalue: &mut Rvalue<'tcx>) {
if let Rvalue::Cast(kind, operand, cast_ty) = rvalue {
let operand_ty = operand.ty(self.local_decls, self.tcx);
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 @@ -1581,6 +1581,7 @@ symbols! {
rustc_test_marker,
rustc_then_this_would_need,
rustc_trivial_field_reads,
rustc_ub_check,
rustc_unsafe_specialization_marker,
rustc_variance,
rustc_variance_of_opaques,
Expand Down
1 change: 1 addition & 0 deletions library/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@
))]
#![no_core]
#![rustc_coherence_is_core]
#![cfg_attr(not(bootstrap), rustc_ub_check)]
//
// Lints:
#![deny(rust_2021_incompatible_or_patterns)]
Expand Down
1 change: 1 addition & 0 deletions library/std/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@
//
#![cfg_attr(not(feature = "restricted-std"), stable(feature = "rust1", since = "1.0.0"))]
#![cfg_attr(feature = "restricted-std", unstable(feature = "restricted_std", issue = "none"))]
#![cfg_attr(not(bootstrap), rustc_ub_check)]
#![doc(
html_playground_url = "https://play.rust-lang.org/",
issue_tracker_base_url = "https://github.com/rust-lang/rust/issues/",
Expand Down
16 changes: 16 additions & 0 deletions tests/mir-opt/instsimplify/ub_check.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//@ unit-test: InstSimplify
//@ compile-flags: -Cdebug-assertions=no -Zinline-mir

// EMIT_MIR ub_check.unwrap_unchecked.InstSimplify.diff
pub fn unwrap_unchecked(x: Option<i32>) -> i32 {
// CHECK-LABEL: fn unwrap_unchecked(
// CHECK-NOT: UbCheck(LanguageUb)
// CHECK: [[assume:_.*]] = const false;
// CHECK-NEXT: assume([[assume]]);
// CHECK-NEXT: unreachable_unchecked::precondition_check
unsafe { x.unwrap_unchecked() }
}

fn main() {
unwrap_unchecked(None);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
- // MIR for `unwrap_unchecked` before InstSimplify
+ // MIR for `unwrap_unchecked` after InstSimplify

fn unwrap_unchecked(_1: Option<i32>) -> i32 {
debug x => _1;
let mut _0: i32;
let mut _2: std::option::Option<i32>;
scope 1 {
scope 2 (inlined #[track_caller] Option::<i32>::unwrap_unchecked) {
debug self => _2;
let mut _3: isize;
scope 3 {
debug val => _0;
}
scope 4 {
scope 5 (inlined unreachable_unchecked) {
let mut _4: bool;
let _5: ();
scope 6 {
}
}
}
}
}

bb0: {
StorageLive(_2);
_2 = _1;
StorageLive(_3);
StorageLive(_5);
_3 = discriminant(_2);
switchInt(move _3) -> [0: bb2, 1: bb3, otherwise: bb1];
}

bb1: {
unreachable;
}

bb2: {
StorageLive(_4);
- _4 = UbCheck(LanguageUb);
+ _4 = const false;
assume(_4);
_5 = unreachable_unchecked::precondition_check() -> [return: bb1, unwind unreachable];
}

bb3: {
_0 = move ((_2 as Some).0: i32);
StorageDead(_5);
StorageDead(_3);
StorageDead(_2);
return;
}
}

0 comments on commit acc30d0

Please sign in to comment.