From 851fcc7a54262748b1aa9e16de91453998d896f3 Mon Sep 17 00:00:00 2001 From: Erik Desjardins Date: Sat, 26 Feb 2022 12:52:07 -0500 Subject: [PATCH 1/2] Revert "Auto merge of #92419 - erikdesjardins:coldland, r=nagisa" This reverts commit 4f49627c6fe2a32d1fed6310466bb0e1c535c0c0, reversing changes made to 028c6f1454787c068ff5117e9000a1de4fd98374. --- compiler/rustc_codegen_gcc/src/builder.rs | 2 +- compiler/rustc_codegen_llvm/src/builder.rs | 17 ++------- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 2 - compiler/rustc_codegen_llvm/src/llvm_util.rs | 6 --- compiler/rustc_codegen_ssa/src/mir/block.rs | 10 +++-- .../rustc_codegen_ssa/src/traits/builder.rs | 2 +- .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 8 ---- src/test/codegen/unwind-landingpad-cold.rs | 15 -------- src/test/codegen/unwind-landingpad-inline.rs | 37 ------------------- 9 files changed, 12 insertions(+), 87 deletions(-) delete mode 100644 src/test/codegen/unwind-landingpad-cold.rs delete mode 100644 src/test/codegen/unwind-landingpad-inline.rs diff --git a/compiler/rustc_codegen_gcc/src/builder.rs b/compiler/rustc_codegen_gcc/src/builder.rs index b430dc329cb9a..974e59b65ec91 100644 --- a/compiler/rustc_codegen_gcc/src/builder.rs +++ b/compiler/rustc_codegen_gcc/src/builder.rs @@ -1403,7 +1403,7 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> { self.cx } - fn apply_attrs_to_cleanup_callsite(&mut self, _llret: RValue<'gcc>) { + fn do_not_inline(&mut self, _llret: RValue<'gcc>) { unimplemented!(); } diff --git a/compiler/rustc_codegen_llvm/src/builder.rs b/compiler/rustc_codegen_llvm/src/builder.rs index 5e78d6fc851a5..3703e902bb7d8 100644 --- a/compiler/rustc_codegen_llvm/src/builder.rs +++ b/compiler/rustc_codegen_llvm/src/builder.rs @@ -23,7 +23,6 @@ use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_span::Span; use rustc_target::abi::{self, call::FnAbi, Align, Size, WrappingRange}; use rustc_target::spec::{HasTargetSpec, Target}; -use smallvec::SmallVec; use std::borrow::Cow; use std::ffi::CStr; use std::iter; @@ -1175,19 +1174,9 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { unsafe { llvm::LLVMBuildZExt(self.llbuilder, val, dest_ty, UNNAMED) } } - fn apply_attrs_to_cleanup_callsite(&mut self, llret: &'ll Value) { - let mut attrs = SmallVec::<[_; 2]>::new(); - - // Cleanup is always the cold path. - attrs.push(llvm::AttributeKind::Cold.create_attr(self.llcx)); - - // In LLVM versions with deferred inlining (currently, system LLVM < 14), - // inlining drop glue can lead to exponential size blowup, see #41696 and #92110. - if !llvm_util::is_rust_llvm() && llvm_util::get_version() < (14, 0, 0) { - attrs.push(llvm::AttributeKind::NoInline.create_attr(self.llcx)); - } - - attributes::apply_to_callsite(llret, llvm::AttributePlace::Function, &attrs); + fn do_not_inline(&mut self, llret: &'ll Value) { + let noinline = llvm::AttributeKind::NoInline.create_attr(self.llcx); + attributes::apply_to_callsite(llret, llvm::AttributePlace::Function, &[noinline]); } } diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 31d1460e178ba..93eddb7359f66 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -1932,8 +1932,6 @@ extern "C" { pub fn LLVMRustVersionMinor() -> u32; pub fn LLVMRustVersionPatch() -> u32; - pub fn LLVMRustIsRustLLVM() -> bool; - /// Add LLVM module flags. /// /// In order for Rust-C LTO to work, module flags must be compatible with Clang. What diff --git a/compiler/rustc_codegen_llvm/src/llvm_util.rs b/compiler/rustc_codegen_llvm/src/llvm_util.rs index b1c14c7e44bd0..6c974eb9c0b1e 100644 --- a/compiler/rustc_codegen_llvm/src/llvm_util.rs +++ b/compiler/rustc_codegen_llvm/src/llvm_util.rs @@ -257,12 +257,6 @@ pub fn get_version() -> (u32, u32, u32) { } } -/// Returns `true` if this LLVM is Rust's bundled LLVM (and not system LLVM). -pub fn is_rust_llvm() -> bool { - // Can be called without initializing LLVM - unsafe { llvm::LLVMRustIsRustLLVM() } -} - pub fn print_passes() { // Can be called without initializing LLVM unsafe { diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index f6c41af41b4d3..a87daa8d6b80b 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -166,7 +166,7 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> { bx.invoke(fn_ty, fn_ptr, &llargs, ret_llbb, unwind_block, self.funclet(fx)); bx.apply_attrs_callsite(&fn_abi, invokeret); if fx.mir[self.bb].is_cleanup { - bx.apply_attrs_to_cleanup_callsite(invokeret); + bx.do_not_inline(invokeret); } if let Some((ret_dest, target)) = destination { @@ -178,7 +178,11 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> { let llret = bx.call(fn_ty, fn_ptr, &llargs, self.funclet(fx)); bx.apply_attrs_callsite(&fn_abi, llret); if fx.mir[self.bb].is_cleanup { - bx.apply_attrs_to_cleanup_callsite(llret); + // Cleanup is always the cold path. Don't inline + // drop glue. Also, when there is a deeply-nested + // struct, there are "symmetry" issues that cause + // exponential inlining - see issue #41696. + bx.do_not_inline(llret); } if let Some((ret_dest, target)) = destination { @@ -1444,7 +1448,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let llret = bx.call(fn_ty, fn_ptr, &[], None); bx.apply_attrs_callsite(&fn_abi, llret); - bx.apply_attrs_to_cleanup_callsite(llret); + bx.do_not_inline(llret); bx.unreachable(); diff --git a/compiler/rustc_codegen_ssa/src/traits/builder.rs b/compiler/rustc_codegen_ssa/src/traits/builder.rs index 1c7fe060ea4fb..37f2bfd3c4fbd 100644 --- a/compiler/rustc_codegen_ssa/src/traits/builder.rs +++ b/compiler/rustc_codegen_ssa/src/traits/builder.rs @@ -479,5 +479,5 @@ pub trait BuilderMethods<'a, 'tcx>: ) -> Self::Value; fn zext(&mut self, val: Self::Value, dest_ty: Self::Type) -> Self::Value; - fn apply_attrs_to_cleanup_callsite(&mut self, llret: Self::Value); + fn do_not_inline(&mut self, llret: Self::Value); } diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index d627af48ba58e..be3c1b02a05ae 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -694,14 +694,6 @@ extern "C" uint32_t LLVMRustVersionMinor() { return LLVM_VERSION_MINOR; } extern "C" uint32_t LLVMRustVersionMajor() { return LLVM_VERSION_MAJOR; } -extern "C" bool LLVMRustIsRustLLVM() { -#ifdef LLVM_RUSTLLVM - return true; -#else - return false; -#endif -} - extern "C" void LLVMRustAddModuleFlag( LLVMModuleRef M, Module::ModFlagBehavior MergeBehavior, diff --git a/src/test/codegen/unwind-landingpad-cold.rs b/src/test/codegen/unwind-landingpad-cold.rs deleted file mode 100644 index aa00b7936541e..0000000000000 --- a/src/test/codegen/unwind-landingpad-cold.rs +++ /dev/null @@ -1,15 +0,0 @@ -// no-system-llvm: needs #92110 -// compile-flags: -Cno-prepopulate-passes -#![crate_type = "lib"] - -// This test checks that drop calls in unwind landing pads -// get the `cold` attribute. - -// CHECK-LABEL: @check_cold -// CHECK: {{(call|invoke) void .+}}drop_in_place{{.+}} [[ATTRIBUTES:#[0-9]+]] -// CHECK: attributes [[ATTRIBUTES]] = { cold } -#[no_mangle] -pub fn check_cold(f: fn(), x: Box) { - // this may unwind - f(); -} diff --git a/src/test/codegen/unwind-landingpad-inline.rs b/src/test/codegen/unwind-landingpad-inline.rs deleted file mode 100644 index ce78d075dd0f4..0000000000000 --- a/src/test/codegen/unwind-landingpad-inline.rs +++ /dev/null @@ -1,37 +0,0 @@ -// no-system-llvm: needs #92110 + patch for Rust alloc/dealloc functions -// compile-flags: -Copt-level=3 -#![crate_type = "lib"] - -// This test checks that we can inline drop_in_place in -// unwind landing pads. - -// Without inlining, the box pointers escape via the call to drop_in_place, -// and LLVM will not optimize out the pointer comparison. -// With inlining, everything should be optimized out. -// See https://github.com/rust-lang/rust/issues/46515 -// CHECK-LABEL: @check_no_escape_in_landingpad -// CHECK: start: -// CHECK-NEXT: ret void -#[no_mangle] -pub fn check_no_escape_in_landingpad(f: fn()) { - let x = &*Box::new(0); - let y = &*Box::new(0); - - if x as *const _ == y as *const _ { - f(); - } -} - -// Without inlining, the compiler can't tell that -// dropping an empty string (in a landing pad) does nothing. -// With inlining, the landing pad should be optimized out. -// See https://github.com/rust-lang/rust/issues/87055 -// CHECK-LABEL: @check_eliminate_noop_drop -// CHECK: start: -// CHECK-NEXT: call void %g() -// CHECK-NEXT: ret void -#[no_mangle] -pub fn check_eliminate_noop_drop(g: fn()) { - let _var = String::new(); - g(); -} From 0c784337494223441d53129688bd777ae8df7992 Mon Sep 17 00:00:00 2001 From: Erik Desjardins Date: Sun, 27 Feb 2022 23:15:49 -0500 Subject: [PATCH 2/2] update vec-shrink-panik test to allow panic_no_unwind in landingpads --- src/test/codegen/vec-shrink-panik.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/test/codegen/vec-shrink-panik.rs b/src/test/codegen/vec-shrink-panik.rs index 6b0ac78857a0e..18409014bdede 100644 --- a/src/test/codegen/vec-shrink-panik.rs +++ b/src/test/codegen/vec-shrink-panik.rs @@ -16,6 +16,14 @@ pub fn shrink_to_fit(vec: &mut Vec) { // CHECK-LABEL: @issue71861 #[no_mangle] pub fn issue71861(vec: Vec) -> Box<[u32]> { + // CHECK-NOT: panic + + // Call to panic_no_unwind in case of double-panic is expected, + // but other panics are not. + // CHECK: cleanup + // CHECK-NEXT: ; call core::panicking::panic_no_unwind + // CHECK-NEXT: panic_no_unwind + // CHECK-NOT: panic vec.into_boxed_slice() } @@ -23,6 +31,17 @@ pub fn issue71861(vec: Vec) -> Box<[u32]> { // CHECK-LABEL: @issue75636 #[no_mangle] pub fn issue75636<'a>(iter: &[&'a str]) -> Box<[&'a str]> { + // CHECK-NOT: panic + + // Call to panic_no_unwind in case of double-panic is expected, + // but other panics are not. + // CHECK: cleanup + // CHECK-NEXT: ; call core::panicking::panic_no_unwind + // CHECK-NEXT: panic_no_unwind + // CHECK-NOT: panic iter.iter().copied().collect() } + +// CHECK: ; core::panicking::panic_no_unwind +// CHECK: declare void @{{.*}}panic_no_unwind