From 783301298f1e151b82c1be21bc6a214b9ef525bd Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 20 Oct 2022 12:28:31 +0200 Subject: Don't use usub.with.overflow intrinsic The canonical form of a usub.with.overflow check in LLVM are separate sub + icmp instructions, rather than a usub.with.overflow intrinsic. Using usub.with.overflow will generally result in worse optimization potential. The backend will attempt to form usub.with.overflow when it comes to actual instruction selection. This is not fully reliable, but I believe this is a better tradeoff than using the intrinsic in IR. Fixes #103285. --- src/test/codegen/issue-103285-ptr-addr-overflow-check.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 src/test/codegen/issue-103285-ptr-addr-overflow-check.rs (limited to 'src/test/codegen') diff --git a/src/test/codegen/issue-103285-ptr-addr-overflow-check.rs b/src/test/codegen/issue-103285-ptr-addr-overflow-check.rs new file mode 100644 index 00000000000..a3499babea2 --- /dev/null +++ b/src/test/codegen/issue-103285-ptr-addr-overflow-check.rs @@ -0,0 +1,16 @@ +// compile-flags: -O -C debug-assertions=yes + +#![crate_type = "lib"] +#![feature(strict_provenance)] + +#[no_mangle] +pub fn test(src: *const u8, dst: *const u8) -> usize { + // CHECK-LABEL: @test( + // CHECK-NOT: panic + let src_usize = src.addr(); + let dst_usize = dst.addr(); + if src_usize > dst_usize { + return src_usize - dst_usize; + } + return 0; +} -- cgit 1.4.1-3-g733a5 From 003a3f8cd3ea4594e2c2bc89c05562a202868ce0 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 20 Oct 2022 18:59:07 +1100 Subject: Use `br` instead of `switch` in more cases. `codegen_switchint_terminator` already uses `br` instead of `switch` when there is one normal target plus the `otherwise` target. But there's another common case with two normal targets and an `otherwise` target that points to an empty unreachable BB. This comes up a lot when switching on the tags of enums that use niches. The pattern looks like this: ``` bb1: ; preds = %bb6 %3 = load i8, ptr %_2, align 1, !range !9, !noundef !4 %4 = sub i8 %3, 2 %5 = icmp eq i8 %4, 0 %_6 = select i1 %5, i64 0, i64 1 switch i64 %_6, label %bb3 [ i64 0, label %bb4 i64 1, label %bb2 ] bb3: ; preds = %bb1 unreachable ``` This commit adds code to convert the `switch` to a `br`: ``` bb1: ; preds = %bb6 %3 = load i8, ptr %_2, align 1, !range !9, !noundef !4 %4 = sub i8 %3, 2 %5 = icmp eq i8 %4, 0 %_6 = select i1 %5, i64 0, i64 1 %6 = icmp eq i64 %_6, 0 br i1 %6, label %bb4, label %bb2 bb3: ; No predecessors! unreachable ``` This has a surprisingly large effect on compile times, with reductions of 5% on debug builds of some crates. The reduction is all due to LLVM taking less time. Maybe LLVM is just much better at handling `br` than `switch`. The resulting code is still suboptimal. - The `icmp`, `select`, `icmp` sequence is silly, converting an `i1` to an `i64` and back to an `i1`. But with the current code structure it's hard to avoid, and LLVM will easily clean it up, in opt builds at least. - `bb3` is usually now truly dead code (though not always, so it can't be removed universally). --- compiler/rustc_codegen_ssa/src/mir/block.rs | 30 ++++++++++++++- compiler/rustc_middle/src/mir/mod.rs | 5 +++ src/test/codegen/match-optimized.rs | 60 +++++++++++++++++++++++++++++ src/test/codegen/match-unoptimized.rs | 23 +++++++++++ src/test/codegen/match.rs | 29 -------------- 5 files changed, 116 insertions(+), 31 deletions(-) create mode 100644 src/test/codegen/match-optimized.rs create mode 100644 src/test/codegen/match-unoptimized.rs delete mode 100644 src/test/codegen/match.rs (limited to 'src/test/codegen') diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 29b7c9b0a88..0802067cde6 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -17,6 +17,7 @@ use rustc_middle::mir::{self, AssertKind, SwitchTargets}; use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf}; use rustc_middle::ty::print::{with_no_trimmed_paths, with_no_visible_paths}; use rustc_middle::ty::{self, Instance, Ty, TypeVisitable}; +use rustc_session::config::OptLevel; use rustc_span::source_map::Span; use rustc_span::{sym, Symbol}; use rustc_symbol_mangling::typeid::typeid_for_fnabi; @@ -286,12 +287,13 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { assert_eq!(discr.layout.ty, switch_ty); let mut target_iter = targets.iter(); if target_iter.len() == 1 { - // If there are two targets (one conditional, one fallback), emit br instead of switch + // If there are two targets (one conditional, one fallback), emit `br` instead of + // `switch`. let (test_value, target) = target_iter.next().unwrap(); let lltrue = helper.llbb_with_cleanup(self, target); let llfalse = helper.llbb_with_cleanup(self, targets.otherwise()); if switch_ty == bx.tcx().types.bool { - // Don't generate trivial icmps when switching on bool + // Don't generate trivial icmps when switching on bool. match test_value { 0 => bx.cond_br(discr.immediate(), llfalse, lltrue), 1 => bx.cond_br(discr.immediate(), lltrue, llfalse), @@ -303,6 +305,30 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let cmp = bx.icmp(IntPredicate::IntEQ, discr.immediate(), llval); bx.cond_br(cmp, lltrue, llfalse); } + } else if self.cx.sess().opts.optimize == OptLevel::No + && target_iter.len() == 2 + && self.mir[targets.otherwise()].is_empty_unreachable() + { + // In unoptimized builds, if there are two normal targets and the `otherwise` target is + // an unreachable BB, emit `br` instead of `switch`. This leaves behind the unreachable + // BB, which will usually (but not always) be dead code. + // + // Why only in unoptimized builds? + // - In unoptimized builds LLVM uses FastISel which does not support switches, so it + // must fall back to the to the slower SelectionDAG isel. Therefore, using `br` gives + // significant compile time speedups for unoptimized builds. + // - In optimized builds the above doesn't hold, and using `br` sometimes results in + // worse generated code because LLVM can no longer tell that the value being switched + // on can only have two values, e.g. 0 and 1. + // + let (test_value1, target1) = target_iter.next().unwrap(); + let (_test_value2, target2) = target_iter.next().unwrap(); + let ll1 = helper.llbb_with_cleanup(self, target1); + let ll2 = helper.llbb_with_cleanup(self, target2); + let switch_llty = bx.immediate_backend_type(bx.layout_of(switch_ty)); + let llval = bx.const_uint_big(switch_llty, test_value1); + let cmp = bx.icmp(IntPredicate::IntEQ, discr.immediate(), llval); + bx.cond_br(cmp, ll1, ll2); } else { bx.switch( discr.immediate(), diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 79db35a764a..068daaadbda 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1186,6 +1186,11 @@ impl<'tcx> BasicBlockData<'tcx> { pub fn visitable(&self, index: usize) -> &dyn MirVisitable<'tcx> { if index < self.statements.len() { &self.statements[index] } else { &self.terminator } } + + /// Does the block have no statements and an unreachable terminator? + pub fn is_empty_unreachable(&self) -> bool { + self.statements.is_empty() && matches!(self.terminator().kind, TerminatorKind::Unreachable) + } } impl AssertKind { diff --git a/src/test/codegen/match-optimized.rs b/src/test/codegen/match-optimized.rs new file mode 100644 index 00000000000..36402cc7353 --- /dev/null +++ b/src/test/codegen/match-optimized.rs @@ -0,0 +1,60 @@ +// compile-flags: -C no-prepopulate-passes -O + +#![crate_type = "lib"] + +pub enum E { + A, + B, + C, +} + +// CHECK-LABEL: @exhaustive_match +#[no_mangle] +pub fn exhaustive_match(e: E) -> u8 { +// CHECK: switch{{.*}}, label %[[OTHERWISE:[a-zA-Z0-9_]+]] [ +// CHECK-NEXT: i[[TY:[0-9]+]] [[DISCR:[0-9]+]], label %[[A:[a-zA-Z0-9_]+]] +// CHECK-NEXT: i[[TY:[0-9]+]] [[DISCR:[0-9]+]], label %[[B:[a-zA-Z0-9_]+]] +// CHECK-NEXT: i[[TY:[0-9]+]] [[DISCR:[0-9]+]], label %[[C:[a-zA-Z0-9_]+]] +// CHECK-NEXT: ] +// CHECK: [[OTHERWISE]]: +// CHECK-NEXT: unreachable +// +// CHECK: [[A]]: +// CHECK-NEXT: store i8 0, {{i8\*|ptr}} %1, align 1 +// CHECK-NEXT: br label %[[EXIT:[a-zA-Z0-9_]+]] +// CHECK: [[B]]: +// CHECK-NEXT: store i8 1, {{i8\*|ptr}} %1, align 1 +// CHECK-NEXT: br label %[[EXIT]] +// CHECK: [[C]]: +// CHECK-NEXT: store i8 2, {{i8\*|ptr}} %1, align 1 +// CHECK-NEXT: br label %[[EXIT]] + match e { + E::A => 0, + E::B => 1, + E::C => 2, + } +} + +#[repr(u16)] +pub enum E2 { + A = 13, + B = 42, +} + +// For optimized code we produce a switch with an unreachable target as the `otherwise` so LLVM +// knows the possible values. Compare with `src/test/codegen/match-unoptimized.rs`. + +// CHECK-LABEL: @exhaustive_match_2 +#[no_mangle] +pub fn exhaustive_match_2(e: E2) -> u8 { + // CHECK: switch i16 %{{.+}}, label %[[UNREACH:.+]] [ + // CHECK-NEXT: i16 13, + // CHECK-NEXT: i16 42, + // CHECK-NEXT: ] + // CHECK: [[UNREACH]]: + // CHECK-NEXT: unreachable + match e { + E2::A => 0, + E2::B => 1, + } +} diff --git a/src/test/codegen/match-unoptimized.rs b/src/test/codegen/match-unoptimized.rs new file mode 100644 index 00000000000..be40b29e3d3 --- /dev/null +++ b/src/test/codegen/match-unoptimized.rs @@ -0,0 +1,23 @@ +// compile-flags: -C no-prepopulate-passes -Copt-level=0 + +#![crate_type = "lib"] + +#[repr(u16)] +pub enum E2 { + A = 13, + B = 42, +} + +// For unoptimized code we produce a `br` instead of a `switch`. Compare with +// `src/test/codegen/match-optimized.rs` + +// CHECK-LABEL: @exhaustive_match_2 +#[no_mangle] +pub fn exhaustive_match_2(e: E2) -> u8 { + // CHECK: %[[CMP:.+]] = icmp eq i16 %{{.+}}, 13 + // CHECK-NEXT: br i1 %[[CMP:.+]], + match e { + E2::A => 0, + E2::B => 1, + } +} diff --git a/src/test/codegen/match.rs b/src/test/codegen/match.rs deleted file mode 100644 index b203641fddb..00000000000 --- a/src/test/codegen/match.rs +++ /dev/null @@ -1,29 +0,0 @@ -// compile-flags: -C no-prepopulate-passes - -#![crate_type = "lib"] - -pub enum E { - A, - B, -} - -// CHECK-LABEL: @exhaustive_match -#[no_mangle] -pub fn exhaustive_match(e: E) -> u8 { -// CHECK: switch{{.*}}, label %[[OTHERWISE:[a-zA-Z0-9_]+]] [ -// CHECK-NEXT: i[[TY:[0-9]+]] [[DISCR:[0-9]+]], label %[[A:[a-zA-Z0-9_]+]] -// CHECK-NEXT: i[[TY:[0-9]+]] [[DISCR:[0-9]+]], label %[[B:[a-zA-Z0-9_]+]] -// CHECK-NEXT: ] -// CHECK: [[OTHERWISE]]: -// CHECK-NEXT: unreachable -// CHECK: [[A]]: -// CHECK-NEXT: store i8 0, {{i8\*|ptr}} %1, align 1 -// CHECK-NEXT: br label %[[EXIT:[a-zA-Z0-9_]+]] -// CHECK: [[B]]: -// CHECK-NEXT: store i8 1, {{i8\*|ptr}} %1, align 1 -// CHECK-NEXT: br label %[[EXIT:[a-zA-Z0-9_]+]] - match e { - E::A => 0, - E::B => 1, - } -} -- cgit 1.4.1-3-g733a5 From a1672ad5b82755b94c0a72038f50b4ba8c25fe81 Mon Sep 17 00:00:00 2001 From: ouz-a Date: Wed, 26 Oct 2022 17:50:11 +0300 Subject: Remove bounds check with enum cast --- .../rustc_mir_build/src/build/expr/as_rvalue.rs | 51 +++++++++++++++++++++- compiler/rustc_mir_transform/src/check_unsafety.rs | 4 +- src/test/codegen/enum-bounds-check-derived-idx.rs | 6 +-- src/test/codegen/enum-bounds-check-issue-13926.rs | 3 +- src/test/codegen/enum-bounds-check.rs | 3 +- .../mir-opt/building/enum_cast.bar.built.after.mir | 6 +++ .../mir-opt/building/enum_cast.boo.built.after.mir | 6 +++ .../building/enum_cast.droppy.built.after.mir | 20 ++++++--- .../self-in-enum-definition.stderr | 40 +++++++++++++++++ 9 files changed, 120 insertions(+), 19 deletions(-) (limited to 'src/test/codegen') diff --git a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs index 3dafdcb7887..5c82fb1ddc0 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs @@ -2,6 +2,7 @@ use rustc_index::vec::Idx; use rustc_middle::ty::util::IntTypeExt; +use rustc_target::abi::{Abi, Primitive}; use crate::build::expr::as_place::PlaceBase; use crate::build::expr::category::{Category, RvalueFunc}; @@ -198,6 +199,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let (source, ty) = if let ty::Adt(adt_def, ..) = source.ty.kind() && adt_def.is_enum() { let discr_ty = adt_def.repr().discr_type().to_ty(this.tcx); let temp = unpack!(block = this.as_temp(block, scope, source, Mutability::Not)); + let layout = this.tcx.layout_of(this.param_env.and(source.ty)); let discr = this.temp(discr_ty, source.span); this.cfg.push_assign( block, @@ -205,8 +207,55 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { discr, Rvalue::Discriminant(temp.into()), ); + let (op,ty) = (Operand::Move(discr), discr_ty); + + if let Abi::Scalar(scalar) = layout.unwrap().abi{ + if let Primitive::Int(_, signed) = scalar.primitive() { + let range = scalar.valid_range(&this.tcx); + // FIXME: Handle wraparound cases too. + if range.end >= range.start { + let mut assumer = |range: u128, bin_op: BinOp| { + // We will be overwriting this val if our scalar is signed value + // because sign extension on unsigned types might cause unintended things + let mut range_val = + ConstantKind::from_bits(this.tcx, range, ty::ParamEnv::empty().and(discr_ty)); + let bool_ty = this.tcx.types.bool; + if signed { + let scalar_size_extend = scalar.size(&this.tcx).sign_extend(range); + let discr_layout = this.tcx.layout_of(this.param_env.and(discr_ty)); + let truncated_val = discr_layout.unwrap().size.truncate(scalar_size_extend); + range_val = ConstantKind::from_bits( + this.tcx, + truncated_val, + ty::ParamEnv::empty().and(discr_ty), + ); + } + let lit_op = this.literal_operand(expr.span, range_val); + let is_bin_op = this.temp(bool_ty, expr_span); + this.cfg.push_assign( + block, + source_info, + is_bin_op, + Rvalue::BinaryOp(bin_op, Box::new(((lit_op), (Operand::Copy(discr))))), + ); + this.cfg.push( + block, + Statement { + source_info, + kind: StatementKind::Intrinsic(Box::new(NonDivergingIntrinsic::Assume( + Operand::Copy(is_bin_op), + ))), + }, + ) + }; + assumer(range.end, BinOp::Ge); + assumer(range.start, BinOp::Le); + } + } + } + + (op,ty) - (Operand::Move(discr), discr_ty) } else { let ty = source.ty; let source = unpack!( diff --git a/compiler/rustc_mir_transform/src/check_unsafety.rs b/compiler/rustc_mir_transform/src/check_unsafety.rs index f8f04214a2c..959fcf8d89e 100644 --- a/compiler/rustc_mir_transform/src/check_unsafety.rs +++ b/compiler/rustc_mir_transform/src/check_unsafety.rs @@ -101,12 +101,10 @@ impl<'tcx> Visitor<'tcx> for UnsafetyChecker<'_, 'tcx> { | StatementKind::Retag { .. } | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) + | StatementKind::Intrinsic(..) | StatementKind::Nop => { // safe (at least as emitted during MIR construction) } - - // Move to above list once mir construction uses it. - StatementKind::Intrinsic(..) => unreachable!(), } self.super_statement(statement, location); } diff --git a/src/test/codegen/enum-bounds-check-derived-idx.rs b/src/test/codegen/enum-bounds-check-derived-idx.rs index fe02aeb5f62..aa66c2ed08e 100644 --- a/src/test/codegen/enum-bounds-check-derived-idx.rs +++ b/src/test/codegen/enum-bounds-check-derived-idx.rs @@ -12,15 +12,13 @@ pub enum Bar { // CHECK-LABEL: @lookup_inc #[no_mangle] pub fn lookup_inc(buf: &[u8; 5], f: Bar) -> u8 { - // FIXME: panic check can be removed by adding the assumes back after https://github.com/rust-lang/rust/pull/98332 - // CHECK: panic_bounds_check + // CHECK-NOT: panic_bounds_check buf[f as usize + 1] } // CHECK-LABEL: @lookup_dec #[no_mangle] pub fn lookup_dec(buf: &[u8; 5], f: Bar) -> u8 { - // FIXME: panic check can be removed by adding the assumes back after https://github.com/rust-lang/rust/pull/98332 - // CHECK: panic_bounds_check + // CHECK-NOT: panic_bounds_check buf[f as usize - 1] } diff --git a/src/test/codegen/enum-bounds-check-issue-13926.rs b/src/test/codegen/enum-bounds-check-issue-13926.rs index 1aec41d5441..b26945bc549 100644 --- a/src/test/codegen/enum-bounds-check-issue-13926.rs +++ b/src/test/codegen/enum-bounds-check-issue-13926.rs @@ -13,7 +13,6 @@ pub enum Exception { // CHECK-LABEL: @access #[no_mangle] pub fn access(array: &[usize; 12], exc: Exception) -> usize { - // FIXME: panic check can be removed by adding the assumes back after https://github.com/rust-lang/rust/pull/98332 - // CHECK: panic_bounds_check + // CHECK-NOT: panic_bounds_check array[(exc as u8 - 4) as usize] } diff --git a/src/test/codegen/enum-bounds-check.rs b/src/test/codegen/enum-bounds-check.rs index f85c6817ded..17322d5911b 100644 --- a/src/test/codegen/enum-bounds-check.rs +++ b/src/test/codegen/enum-bounds-check.rs @@ -21,7 +21,6 @@ pub enum Bar { // CHECK-LABEL: @lookup_unmodified #[no_mangle] pub fn lookup_unmodified(buf: &[u8; 5], f: Bar) -> u8 { - // FIXME: panic check can be removed by adding the assumes back after https://github.com/rust-lang/rust/pull/98332 - // CHECK: panic_bounds_check + // CHECK-NOT: panic_bounds_check buf[f as usize] } diff --git a/src/test/mir-opt/building/enum_cast.bar.built.after.mir b/src/test/mir-opt/building/enum_cast.bar.built.after.mir index 194b107bead..0746e0b498e 100644 --- a/src/test/mir-opt/building/enum_cast.bar.built.after.mir +++ b/src/test/mir-opt/building/enum_cast.bar.built.after.mir @@ -5,11 +5,17 @@ fn bar(_1: Bar) -> usize { let mut _0: usize; // return place in scope 0 at $DIR/enum_cast.rs:+0:21: +0:26 let _2: Bar; // in scope 0 at $DIR/enum_cast.rs:+1:5: +1:8 let mut _3: isize; // in scope 0 at $DIR/enum_cast.rs:+1:5: +1:8 + let mut _4: bool; // in scope 0 at $DIR/enum_cast.rs:+1:5: +1:17 + let mut _5: bool; // in scope 0 at $DIR/enum_cast.rs:+1:5: +1:17 bb0: { StorageLive(_2); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:8 _2 = move _1; // scope 0 at $DIR/enum_cast.rs:+1:5: +1:8 _3 = discriminant(_2); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17 + _4 = Ge(const 1_isize, _3); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17 + assume(_4); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17 + _5 = Le(const 0_isize, _3); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17 + assume(_5); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17 _0 = move _3 as usize (IntToInt); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17 StorageDead(_2); // scope 0 at $DIR/enum_cast.rs:+1:16: +1:17 return; // scope 0 at $DIR/enum_cast.rs:+2:2: +2:2 diff --git a/src/test/mir-opt/building/enum_cast.boo.built.after.mir b/src/test/mir-opt/building/enum_cast.boo.built.after.mir index dde26afc77a..699c876b01a 100644 --- a/src/test/mir-opt/building/enum_cast.boo.built.after.mir +++ b/src/test/mir-opt/building/enum_cast.boo.built.after.mir @@ -5,11 +5,17 @@ fn boo(_1: Boo) -> usize { let mut _0: usize; // return place in scope 0 at $DIR/enum_cast.rs:+0:21: +0:26 let _2: Boo; // in scope 0 at $DIR/enum_cast.rs:+1:5: +1:8 let mut _3: u8; // in scope 0 at $DIR/enum_cast.rs:+1:5: +1:8 + let mut _4: bool; // in scope 0 at $DIR/enum_cast.rs:+1:5: +1:17 + let mut _5: bool; // in scope 0 at $DIR/enum_cast.rs:+1:5: +1:17 bb0: { StorageLive(_2); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:8 _2 = move _1; // scope 0 at $DIR/enum_cast.rs:+1:5: +1:8 _3 = discriminant(_2); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17 + _4 = Ge(const 1_u8, _3); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17 + assume(_4); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17 + _5 = Le(const 0_u8, _3); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17 + assume(_5); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17 _0 = move _3 as usize (IntToInt); // scope 0 at $DIR/enum_cast.rs:+1:5: +1:17 StorageDead(_2); // scope 0 at $DIR/enum_cast.rs:+1:16: +1:17 return; // scope 0 at $DIR/enum_cast.rs:+2:2: +2:2 diff --git a/src/test/mir-opt/building/enum_cast.droppy.built.after.mir b/src/test/mir-opt/building/enum_cast.droppy.built.after.mir index a43c523c71f..5231c2eab95 100644 --- a/src/test/mir-opt/building/enum_cast.droppy.built.after.mir +++ b/src/test/mir-opt/building/enum_cast.droppy.built.after.mir @@ -6,7 +6,9 @@ fn droppy() -> () { let _2: Droppy; // in scope 0 at $DIR/enum_cast.rs:+2:13: +2:14 let _4: Droppy; // in scope 0 at $DIR/enum_cast.rs:+5:17: +5:18 let mut _5: isize; // in scope 0 at $DIR/enum_cast.rs:+5:17: +5:18 - let _6: Droppy; // in scope 0 at $DIR/enum_cast.rs:+7:9: +7:10 + let mut _6: bool; // in scope 0 at $DIR/enum_cast.rs:+5:17: +5:27 + let mut _7: bool; // in scope 0 at $DIR/enum_cast.rs:+5:17: +5:27 + let _8: Droppy; // in scope 0 at $DIR/enum_cast.rs:+7:9: +7:10 scope 1 { debug x => _2; // in scope 1 at $DIR/enum_cast.rs:+2:13: +2:14 scope 2 { @@ -17,7 +19,7 @@ fn droppy() -> () { } } scope 4 { - debug z => _6; // in scope 4 at $DIR/enum_cast.rs:+7:9: +7:10 + debug z => _8; // in scope 4 at $DIR/enum_cast.rs:+7:9: +7:10 } bb0: { @@ -29,6 +31,10 @@ fn droppy() -> () { StorageLive(_4); // scope 3 at $DIR/enum_cast.rs:+5:17: +5:18 _4 = move _2; // scope 3 at $DIR/enum_cast.rs:+5:17: +5:18 _5 = discriminant(_4); // scope 3 at $DIR/enum_cast.rs:+5:17: +5:27 + _6 = Ge(const 2_isize, _5); // scope 3 at $DIR/enum_cast.rs:+5:17: +5:27 + assume(_6); // scope 3 at $DIR/enum_cast.rs:+5:17: +5:27 + _7 = Le(const 0_isize, _5); // scope 3 at $DIR/enum_cast.rs:+5:17: +5:27 + assume(_7); // scope 3 at $DIR/enum_cast.rs:+5:17: +5:27 _3 = move _5 as usize (IntToInt); // scope 3 at $DIR/enum_cast.rs:+5:17: +5:27 drop(_4) -> [return: bb1, unwind: bb4]; // scope 3 at $DIR/enum_cast.rs:+5:26: +5:27 } @@ -44,15 +50,15 @@ fn droppy() -> () { bb2: { StorageDead(_2); // scope 0 at $DIR/enum_cast.rs:+6:5: +6:6 StorageDead(_1); // scope 0 at $DIR/enum_cast.rs:+6:5: +6:6 - StorageLive(_6); // scope 0 at $DIR/enum_cast.rs:+7:9: +7:10 - _6 = Droppy::B; // scope 0 at $DIR/enum_cast.rs:+7:13: +7:22 - FakeRead(ForLet(None), _6); // scope 0 at $DIR/enum_cast.rs:+7:9: +7:10 + StorageLive(_8); // scope 0 at $DIR/enum_cast.rs:+7:9: +7:10 + _8 = Droppy::B; // scope 0 at $DIR/enum_cast.rs:+7:13: +7:22 + FakeRead(ForLet(None), _8); // scope 0 at $DIR/enum_cast.rs:+7:9: +7:10 _0 = const (); // scope 0 at $DIR/enum_cast.rs:+0:13: +8:2 - drop(_6) -> [return: bb3, unwind: bb5]; // scope 0 at $DIR/enum_cast.rs:+8:1: +8:2 + drop(_8) -> [return: bb3, unwind: bb5]; // scope 0 at $DIR/enum_cast.rs:+8:1: +8:2 } bb3: { - StorageDead(_6); // scope 0 at $DIR/enum_cast.rs:+8:1: +8:2 + StorageDead(_8); // scope 0 at $DIR/enum_cast.rs:+8:1: +8:2 return; // scope 0 at $DIR/enum_cast.rs:+8:2: +8:2 } diff --git a/src/test/ui/type-alias-enum-variants/self-in-enum-definition.stderr b/src/test/ui/type-alias-enum-variants/self-in-enum-definition.stderr index 4775e68820b..576fc6a4f8d 100644 --- a/src/test/ui/type-alias-enum-variants/self-in-enum-definition.stderr +++ b/src/test/ui/type-alias-enum-variants/self-in-enum-definition.stderr @@ -12,6 +12,46 @@ LL | V3 = Self::V1 {} as u8 + 2, note: ...which requires const-evaluating + checking `Alpha::V3::{constant#0}`... --> $DIR/self-in-enum-definition.rs:5:10 | +LL | V3 = Self::V1 {} as u8 + 2, + | ^^^^^^^^^^^^^^^^^^^^^ +note: ...which requires caching mir of `Alpha::V3::{constant#0}` for CTFE... + --> $DIR/self-in-enum-definition.rs:5:10 + | +LL | V3 = Self::V1 {} as u8 + 2, + | ^^^^^^^^^^^^^^^^^^^^^ +note: ...which requires elaborating drops for `Alpha::V3::{constant#0}`... + --> $DIR/self-in-enum-definition.rs:5:10 + | +LL | V3 = Self::V1 {} as u8 + 2, + | ^^^^^^^^^^^^^^^^^^^^^ +note: ...which requires borrow-checking `Alpha::V3::{constant#0}`... + --> $DIR/self-in-enum-definition.rs:5:10 + | +LL | V3 = Self::V1 {} as u8 + 2, + | ^^^^^^^^^^^^^^^^^^^^^ +note: ...which requires processing MIR for `Alpha::V3::{constant#0}`... + --> $DIR/self-in-enum-definition.rs:5:10 + | +LL | V3 = Self::V1 {} as u8 + 2, + | ^^^^^^^^^^^^^^^^^^^^^ +note: ...which requires const checking `Alpha::V3::{constant#0}`... + --> $DIR/self-in-enum-definition.rs:5:10 + | +LL | V3 = Self::V1 {} as u8 + 2, + | ^^^^^^^^^^^^^^^^^^^^^ +note: ...which requires preparing `Alpha::V3::{constant#0}` for borrow checking... + --> $DIR/self-in-enum-definition.rs:5:10 + | +LL | V3 = Self::V1 {} as u8 + 2, + | ^^^^^^^^^^^^^^^^^^^^^ +note: ...which requires unsafety-checking `Alpha::V3::{constant#0}`... + --> $DIR/self-in-enum-definition.rs:5:10 + | +LL | V3 = Self::V1 {} as u8 + 2, + | ^^^^^^^^^^^^^^^^^^^^^ +note: ...which requires building MIR for `Alpha::V3::{constant#0}`... + --> $DIR/self-in-enum-definition.rs:5:10 + | LL | V3 = Self::V1 {} as u8 + 2, | ^^^^^^^^^^^^^^^^^^^^^ = note: ...which requires computing layout of `Alpha`... -- cgit 1.4.1-3-g733a5 From c15cfc91c4567c4f079c2543dd395908f30f4911 Mon Sep 17 00:00:00 2001 From: Tim Neumann Date: Fri, 4 Nov 2022 16:20:42 +0000 Subject: LLVM 16: Switch to using MemoryEffects --- compiler/rustc_codegen_llvm/src/asm.rs | 6 ++-- compiler/rustc_codegen_llvm/src/attributes.rs | 6 ++-- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 11 ++++++- compiler/rustc_codegen_llvm/src/llvm/mod.rs | 7 ++++ compiler/rustc_llvm/llvm-wrapper/LLVMWrapper.h | 1 - compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp | 42 ++++++++++++++++++++++-- src/test/codegen/ffi-const.rs | 3 +- src/test/codegen/ffi-pure.rs | 3 +- 8 files changed, 67 insertions(+), 12 deletions(-) (limited to 'src/test/codegen') diff --git a/compiler/rustc_codegen_llvm/src/asm.rs b/compiler/rustc_codegen_llvm/src/asm.rs index 88a4f62d93d..2f22b45e810 100644 --- a/compiler/rustc_codegen_llvm/src/asm.rs +++ b/compiler/rustc_codegen_llvm/src/asm.rs @@ -285,13 +285,13 @@ impl<'ll, 'tcx> AsmBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> { let mut attrs = SmallVec::<[_; 2]>::new(); if options.contains(InlineAsmOptions::PURE) { if options.contains(InlineAsmOptions::NOMEM) { - attrs.push(llvm::AttributeKind::ReadNone.create_attr(self.cx.llcx)); + attrs.push(llvm::MemoryEffects::None.create_attr(self.cx.llcx)); } else if options.contains(InlineAsmOptions::READONLY) { - attrs.push(llvm::AttributeKind::ReadOnly.create_attr(self.cx.llcx)); + attrs.push(llvm::MemoryEffects::ReadOnly.create_attr(self.cx.llcx)); } attrs.push(llvm::AttributeKind::WillReturn.create_attr(self.cx.llcx)); } else if options.contains(InlineAsmOptions::NOMEM) { - attrs.push(llvm::AttributeKind::InaccessibleMemOnly.create_attr(self.cx.llcx)); + attrs.push(llvm::MemoryEffects::InaccessibleMemOnly.create_attr(self.cx.llcx)); } else { // LLVM doesn't have an attribute to represent ReadOnly + SideEffect } diff --git a/compiler/rustc_codegen_llvm/src/attributes.rs b/compiler/rustc_codegen_llvm/src/attributes.rs index eff2436d41c..d96da5cc11d 100644 --- a/compiler/rustc_codegen_llvm/src/attributes.rs +++ b/compiler/rustc_codegen_llvm/src/attributes.rs @@ -13,7 +13,7 @@ use smallvec::SmallVec; use crate::attributes; use crate::llvm::AttributePlace::Function; -use crate::llvm::{self, AllocKindFlags, Attribute, AttributeKind, AttributePlace}; +use crate::llvm::{self, AllocKindFlags, Attribute, AttributeKind, AttributePlace, MemoryEffects}; use crate::llvm_util; pub use rustc_attr::{InlineAttr, InstructionSetAttr, OptimizeAttr}; @@ -303,10 +303,10 @@ pub fn from_fn_attrs<'ll, 'tcx>( to_add.push(AttributeKind::ReturnsTwice.create_attr(cx.llcx)); } if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::FFI_PURE) { - to_add.push(AttributeKind::ReadOnly.create_attr(cx.llcx)); + to_add.push(MemoryEffects::ReadOnly.create_attr(cx.llcx)); } if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::FFI_CONST) { - to_add.push(AttributeKind::ReadNone.create_attr(cx.llcx)); + to_add.push(MemoryEffects::None.create_attr(cx.llcx)); } if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NAKED) { to_add.push(AttributeKind::Naked.create_attr(cx.llcx)); diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 42cb694c0e7..e2d0390821d 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -183,7 +183,6 @@ pub enum AttributeKind { OptimizeNone = 24, ReturnsTwice = 25, ReadNone = 26, - InaccessibleMemOnly = 27, SanitizeHWAddress = 28, WillReturn = 29, StackProtectReq = 30, @@ -590,6 +589,15 @@ pub enum ChecksumKind { SHA256, } +/// LLVMRustMemoryEffects +#[derive(Copy, Clone)] +#[repr(C)] +pub enum MemoryEffects { + None, + ReadOnly, + InaccessibleMemOnly, +} + extern "C" { type Opaque; } @@ -1175,6 +1183,7 @@ extern "C" { pub fn LLVMRustCreateUWTableAttr(C: &Context, async_: bool) -> &Attribute; pub fn LLVMRustCreateAllocSizeAttr(C: &Context, size_arg: u32) -> &Attribute; pub fn LLVMRustCreateAllocKindAttr(C: &Context, size_arg: u64) -> &Attribute; + pub fn LLVMRustCreateMemoryEffectsAttr(C: &Context, effects: MemoryEffects) -> &Attribute; // Operations on functions pub fn LLVMRustGetOrInsertFunction<'a>( diff --git a/compiler/rustc_codegen_llvm/src/llvm/mod.rs b/compiler/rustc_codegen_llvm/src/llvm/mod.rs index 6602a4ab863..f820e752371 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/mod.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/mod.rs @@ -185,6 +185,13 @@ impl AttributeKind { } } +impl MemoryEffects { + /// Create an LLVM Attribute with these memory effects. + pub fn create_attr(self, llcx: &Context) -> &Attribute { + unsafe { LLVMRustCreateMemoryEffectsAttr(llcx, self) } + } +} + pub fn set_section(llglobal: &Value, section_name: &str) { let section_name_cstr = CString::new(section_name).expect("unexpected CString error"); unsafe { diff --git a/compiler/rustc_llvm/llvm-wrapper/LLVMWrapper.h b/compiler/rustc_llvm/llvm-wrapper/LLVMWrapper.h index 015c1c52bef..727cfc4416e 100644 --- a/compiler/rustc_llvm/llvm-wrapper/LLVMWrapper.h +++ b/compiler/rustc_llvm/llvm-wrapper/LLVMWrapper.h @@ -76,7 +76,6 @@ enum LLVMRustAttribute { OptimizeNone = 24, ReturnsTwice = 25, ReadNone = 26, - InaccessibleMemOnly = 27, SanitizeHWAddress = 28, WillReturn = 29, StackProtectReq = 30, diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index 6f36281af23..0d9b5a57b69 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -8,6 +8,9 @@ #include "llvm/IR/Intrinsics.h" #include "llvm/IR/IntrinsicsARM.h" #include "llvm/IR/Mangler.h" +#if LLVM_VERSION_GE(16, 0) +#include "llvm/IR/ModRef.h" +#endif #include "llvm/Object/Archive.h" #include "llvm/Object/COFFImportFile.h" #include "llvm/Object/ObjectFile.h" @@ -213,8 +216,6 @@ static Attribute::AttrKind fromRust(LLVMRustAttribute Kind) { return Attribute::ReturnsTwice; case ReadNone: return Attribute::ReadNone; - case InaccessibleMemOnly: - return Attribute::InaccessibleMemOnly; case SanitizeHWAddress: return Attribute::SanitizeHWAddress; case WillReturn: @@ -379,6 +380,43 @@ extern "C" LLVMAttributeRef LLVMRustCreateAllocKindAttr(LLVMContextRef C, uint64 #endif } +// Simplified representation of `MemoryEffects` across the FFI boundary. +// +// Each variant corresponds to one of the static factory methods on `MemoryEffects`. +enum class LLVMRustMemoryEffects { + None, + ReadOnly, + InaccessibleMemOnly, +}; + +extern "C" LLVMAttributeRef LLVMRustCreateMemoryEffectsAttr(LLVMContextRef C, + LLVMRustMemoryEffects Effects) { +#if LLVM_VERSION_GE(16, 0) + switch (Effects) { + case LLVMRustMemoryEffects::None: + return wrap(Attribute::getWithMemoryEffects(*unwrap(C), MemoryEffects::none())); + case LLVMRustMemoryEffects::ReadOnly: + return wrap(Attribute::getWithMemoryEffects(*unwrap(C), MemoryEffects::readOnly())); + case LLVMRustMemoryEffects::InaccessibleMemOnly: + return wrap(Attribute::getWithMemoryEffects(*unwrap(C), + MemoryEffects::inaccessibleMemOnly())); + default: + report_fatal_error("bad MemoryEffects."); + } +#else + switch (Effects) { + case LLVMRustMemoryEffects::None: + return wrap(Attribute::get(*unwrap(C), Attribute::ReadNone)); + case LLVMRustMemoryEffects::ReadOnly: + return wrap(Attribute::get(*unwrap(C), Attribute::ReadOnly)); + case LLVMRustMemoryEffects::InaccessibleMemOnly: + return wrap(Attribute::get(*unwrap(C), Attribute::InaccessibleMemOnly)); + default: + report_fatal_error("bad MemoryEffects."); + } +#endif +} + // Enable a fast-math flag // // https://llvm.org/docs/LangRef.html#fast-math-flags diff --git a/src/test/codegen/ffi-const.rs b/src/test/codegen/ffi-const.rs index d9cfa5429b5..93720503480 100644 --- a/src/test/codegen/ffi-const.rs +++ b/src/test/codegen/ffi-const.rs @@ -7,6 +7,7 @@ pub fn bar() { unsafe { foo() } } extern "C" { // CHECK-LABEL: declare{{.*}}void @foo() // CHECK-SAME: [[ATTRS:#[0-9]+]] - // CHECK-DAG: attributes [[ATTRS]] = { {{.*}}readnone{{.*}} } + // The attribute changed from `readnone` to `memory(none)` with LLVM 16.0. + // CHECK-DAG: attributes [[ATTRS]] = { {{.*}}{{readnone|memory\(none\)}}{{.*}} } #[ffi_const] pub fn foo(); } diff --git a/src/test/codegen/ffi-pure.rs b/src/test/codegen/ffi-pure.rs index 5bdb2ee912a..2ed73581358 100644 --- a/src/test/codegen/ffi-pure.rs +++ b/src/test/codegen/ffi-pure.rs @@ -7,6 +7,7 @@ pub fn bar() { unsafe { foo() } } extern "C" { // CHECK-LABEL: declare{{.*}}void @foo() // CHECK-SAME: [[ATTRS:#[0-9]+]] - // CHECK-DAG: attributes [[ATTRS]] = { {{.*}}readonly{{.*}} } + // The attribute changed from `readonly` to `memory(read)` with LLVM 16.0. + // CHECK-DAG: attributes [[ATTRS]] = { {{.*}}{{readonly|memory\(read\)}}{{.*}} } #[ffi_pure] pub fn foo(); } -- cgit 1.4.1-3-g733a5