From f71e0daa29b232d8f689f77fecb84dcb87fce6da Mon Sep 17 00:00:00 2001 From: Xiang Fan Date: Thu, 6 Jun 2019 08:39:20 +0800 Subject: [PATCH 1/3] Add llvm.sideeffect to potential infinite loops and recursions LLVM assumes that a thread will eventually cause side effect. This is not true in Rust if a loop or recursion does nothing in its body, causing undefined behavior even in common cases like `loop {}`. Inserting llvm.sideeffect fixes the undefined behavior. As a micro-optimization, only insert llvm.sideeffect when jumping back in blocks or calling a function. A patch for LLVM is expected to allow empty non-terminate code by default and fix this issue from LLVM side. https://github.com/rust-lang/rust/issues/28728 --- src/librustc_codegen_llvm/context.rs | 1 + src/librustc_codegen_llvm/intrinsic.rs | 6 ++++ src/librustc_codegen_ssa/mir/block.rs | 36 ++++++++++++++++++- src/librustc_codegen_ssa/mir/rvalue.rs | 1 + src/librustc_codegen_ssa/traits/intrinsic.rs | 1 + src/test/codegen/alloc-optimisation.rs | 3 +- src/test/codegen/dealloc-no-unwind.rs | 2 +- src/test/codegen/issue-34947-pow-i32.rs | 9 +++-- src/test/codegen/issue-45222.rs | 6 ++++ src/test/codegen/naked-functions.rs | 10 +++--- .../codegen/non-terminate/infinite-loop-1.rs | 17 +++++++++ .../codegen/non-terminate/infinite-loop-2.rs | 19 ++++++++++ .../non-terminate/infinite-recursion.rs | 14 ++++++++ src/test/codegen/repeat-trusted-len.rs | 6 +++- src/test/codegen/vec-clear.rs | 6 ++++ src/test/codegen/vec-iter-collect-len.rs | 4 +-- src/test/codegen/vec-optimizes-away.rs | 2 +- .../inline-always-many-cgu/Makefile | 2 +- 18 files changed, 127 insertions(+), 18 deletions(-) create mode 100644 src/test/codegen/non-terminate/infinite-loop-1.rs create mode 100644 src/test/codegen/non-terminate/infinite-loop-2.rs create mode 100644 src/test/codegen/non-terminate/infinite-recursion.rs diff --git a/src/librustc_codegen_llvm/context.rs b/src/librustc_codegen_llvm/context.rs index 58ce97039099e..bac37369a29a8 100644 --- a/src/librustc_codegen_llvm/context.rs +++ b/src/librustc_codegen_llvm/context.rs @@ -537,6 +537,7 @@ impl CodegenCx<'b, 'tcx> { ifn!("llvm.trap", fn() -> void); ifn!("llvm.debugtrap", fn() -> void); ifn!("llvm.frameaddress", fn(t_i32) -> i8p); + ifn!("llvm.sideeffect", fn() -> void); ifn!("llvm.powi.f32", fn(t_f32, t_i32) -> t_f32); ifn!("llvm.powi.v2f32", fn(t_v2f32, t_i32) -> t_v2f32); diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index b7a410c3760cd..a8734d338df17 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -124,6 +124,7 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { self.call(expect, &[args[0].immediate(), self.const_bool(false)], None) } "try" => { + self.sideeffect(); try_intrinsic(self, args[0].immediate(), args[1].immediate(), @@ -724,6 +725,11 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { self.call(expect, &[cond, self.const_bool(expected)], None) } + fn sideeffect(&mut self) { + let fnname = self.get_intrinsic(&("llvm.sideeffect")); + self.call(fnname, &[], None); + } + fn va_start(&mut self, va_list: &'ll Value) -> &'ll Value { let intrinsic = self.cx().get_intrinsic("llvm.va_start"); self.call(intrinsic, &[va_list], None) diff --git a/src/librustc_codegen_ssa/mir/block.rs b/src/librustc_codegen_ssa/mir/block.rs index 3069199a21256..99d70e6bb8253 100644 --- a/src/librustc_codegen_ssa/mir/block.rs +++ b/src/librustc_codegen_ssa/mir/block.rs @@ -148,6 +148,24 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'a, 'tcx> { } } } + + // Generate sideeffect intrinsic if jumping to any of the targets can form + // a loop. + fn maybe_sideeffect<'b, 'tcx2: 'b, Bx: BuilderMethods<'b, 'tcx2>>( + &self, + mir: &'b mir::Body<'tcx>, + bx: &mut Bx, + targets: &[mir::BasicBlock], + ) { + if targets.iter().any(|target| { + *target <= *self.bb + && target + .start_location() + .is_predecessor_of(self.bb.start_location(), mir) + }) { + bx.sideeffect(); + } + } } /// Codegen implementations for some terminator variants. @@ -196,6 +214,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let lltrue = helper.llblock(self, targets[0]); let llfalse = helper.llblock(self, targets[1]); if switch_ty == bx.tcx().types.bool { + helper.maybe_sideeffect(self.mir, &mut bx, targets.as_slice()); // Don't generate trivial icmps when switching on bool if let [0] = values[..] { bx.cond_br(discr.immediate(), llfalse, lltrue); @@ -209,9 +228,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { ); let llval = bx.const_uint_big(switch_llty, values[0]); let cmp = bx.icmp(IntPredicate::IntEQ, discr.immediate(), llval); + helper.maybe_sideeffect(self.mir, &mut bx, targets.as_slice()); bx.cond_br(cmp, lltrue, llfalse); } } else { + helper.maybe_sideeffect(self.mir, &mut bx, targets.as_slice()); let (otherwise, targets) = targets.split_last().unwrap(); bx.switch( discr.immediate(), @@ -310,6 +331,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { if let ty::InstanceDef::DropGlue(_, None) = drop_fn.def { // we don't actually need to drop anything. + helper.maybe_sideeffect(self.mir, &mut bx, &[target]); helper.funclet_br(self, &mut bx, target); return } @@ -340,6 +362,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { FnType::of_instance(&bx, drop_fn)) } }; + bx.sideeffect(); helper.do_call(self, &mut bx, fn_ty, drop_fn, args, Some((ReturnDest::Nothing, target)), unwind); @@ -375,6 +398,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { // Don't codegen the panic block if success if known. if const_cond == Some(expected) { + helper.maybe_sideeffect(self.mir, &mut bx, &[target]); helper.funclet_br(self, &mut bx, target); return; } @@ -385,6 +409,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { // Create the failure block and the conditional branch to it. let lltarget = helper.llblock(self, target); let panic_block = self.new_block("panic"); + helper.maybe_sideeffect(self.mir, &mut bx, &[target]); if expected { bx.cond_br(cond, lltarget, panic_block.llbb()); } else { @@ -437,6 +462,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let fn_ty = FnType::of_instance(&bx, instance); let llfn = bx.get_fn(instance); + bx.sideeffect(); // Codegen the actual panic invoke/call. helper.do_call(self, &mut bx, fn_ty, llfn, &args, None, cleanup); } @@ -488,6 +514,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { if let Some(destination_ref) = destination.as_ref() { let &(ref dest, target) = destination_ref; self.codegen_transmute(&mut bx, &args[0], dest); + helper.maybe_sideeffect(self.mir, &mut bx, &[target]); helper.funclet_br(self, &mut bx, target); } else { // If we are trying to transmute to an uninhabited type, @@ -518,6 +545,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { Some(ty::InstanceDef::DropGlue(_, None)) => { // Empty drop glue; a no-op. let &(_, target) = destination.as_ref().unwrap(); + helper.maybe_sideeffect(self.mir, &mut bx, &[target]); helper.funclet_br(self, &mut bx, target); return; } @@ -554,6 +582,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let fn_ty = FnType::of_instance(&bx, instance); let llfn = bx.get_fn(instance); + bx.sideeffect(); // Codegen the actual panic invoke/call. helper.do_call( self, @@ -566,7 +595,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { ); } else { // a NOP - helper.funclet_br(self, &mut bx, destination.as_ref().unwrap().1) + let target = destination.as_ref().unwrap().1; + helper.maybe_sideeffect(self.mir, &mut bx, &[target]); + helper.funclet_br(self, &mut bx, target); } return; } @@ -675,6 +706,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { } if let Some((_, target)) = *destination { + helper.maybe_sideeffect(self.mir, &mut bx, &[target]); helper.funclet_br(self, &mut bx, target); } else { bx.unreachable(); @@ -786,6 +818,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { _ => span_bug!(span, "no llfn for call"), }; + bx.sideeffect(); helper.do_call(self, &mut bx, fn_ty, fn_ptr, &llargs, destination.as_ref().map(|&(_, target)| (ret_dest, target)), cleanup); @@ -835,6 +868,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { } mir::TerminatorKind::Goto { target } => { + helper.maybe_sideeffect(self.mir, &mut bx, &[target]); helper.funclet_br(self, &mut bx, target); } diff --git a/src/librustc_codegen_ssa/mir/rvalue.rs b/src/librustc_codegen_ssa/mir/rvalue.rs index 9b69383b455cf..11a75c6f38c7f 100644 --- a/src/librustc_codegen_ssa/mir/rvalue.rs +++ b/src/librustc_codegen_ssa/mir/rvalue.rs @@ -486,6 +486,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { }; let instance = ty::Instance::mono(bx.tcx(), def_id); let r = bx.cx().get_fn(instance); + bx.sideeffect(); let call = bx.call(r, &[llsize, llalign], None); let val = bx.pointercast(call, llty_ptr); diff --git a/src/librustc_codegen_ssa/traits/intrinsic.rs b/src/librustc_codegen_ssa/traits/intrinsic.rs index 7c79cd6021031..2c484084c4a20 100644 --- a/src/librustc_codegen_ssa/traits/intrinsic.rs +++ b/src/librustc_codegen_ssa/traits/intrinsic.rs @@ -20,6 +20,7 @@ pub trait IntrinsicCallMethods<'tcx>: BackendTypes { fn abort(&mut self); fn assume(&mut self, val: Self::Value); fn expect(&mut self, cond: Self::Value, expected: bool) -> Self::Value; + fn sideeffect(&mut self); /// Trait method used to inject `va_start` on the "spoofed" `VaListImpl` in /// Rust defined C-variadic functions. fn va_start(&mut self, val: Self::Value) -> Self::Value; diff --git a/src/test/codegen/alloc-optimisation.rs b/src/test/codegen/alloc-optimisation.rs index c3ffaeb9547b3..c1e36a17ec291 100644 --- a/src/test/codegen/alloc-optimisation.rs +++ b/src/test/codegen/alloc-optimisation.rs @@ -7,7 +7,8 @@ pub fn alloc_test(data: u32) { // CHECK-LABEL: @alloc_test // CHECK-NEXT: start: - // CHECK-NEXT: ret void + // CHECK-NOT: alloc + // CHECK: ret void let x = Box::new(data); drop(x); } diff --git a/src/test/codegen/dealloc-no-unwind.rs b/src/test/codegen/dealloc-no-unwind.rs index ff21b4caa83c3..f8fc663169d9b 100644 --- a/src/test/codegen/dealloc-no-unwind.rs +++ b/src/test/codegen/dealloc-no-unwind.rs @@ -17,7 +17,7 @@ impl Drop for A { pub fn a(a: Box) { // CHECK-LABEL: define void @a // CHECK: call void @__rust_dealloc - // CHECK-NEXT: call void @foo + // CHECK: call void @foo let _a = A; drop(a); } diff --git a/src/test/codegen/issue-34947-pow-i32.rs b/src/test/codegen/issue-34947-pow-i32.rs index 653da8e8b5f7b..bd8b79adf43b6 100644 --- a/src/test/codegen/issue-34947-pow-i32.rs +++ b/src/test/codegen/issue-34947-pow-i32.rs @@ -6,8 +6,11 @@ #[no_mangle] pub fn issue_34947(x: i32) -> i32 { // CHECK: mul - // CHECK-NEXT: mul - // CHECK-NEXT: mul - // CHECK-NEXT: ret + // CHECK-NOT: br label + // CHECK: mul + // CHECK-NOT: br label + // CHECK: mul + // CHECK-NOT: br label + // CHECK: ret x.pow(5) } diff --git a/src/test/codegen/issue-45222.rs b/src/test/codegen/issue-45222.rs index 7aadc8a095498..894927c5c3351 100644 --- a/src/test/codegen/issue-45222.rs +++ b/src/test/codegen/issue-45222.rs @@ -1,3 +1,9 @@ +// ignore-test + +// FIXME: +// LLVM can't optimize some loops with a large number of iterations because of +// @llvm.sideeffect() (see also #59546) + // compile-flags: -O // ignore-debug: the debug assertions get in the way diff --git a/src/test/codegen/naked-functions.rs b/src/test/codegen/naked-functions.rs index 2050193b61b54..fb683a49a8a80 100644 --- a/src/test/codegen/naked-functions.rs +++ b/src/test/codegen/naked-functions.rs @@ -1,5 +1,3 @@ -// ignore-tidy-linelength - // compile-flags: -C no-prepopulate-passes #![crate_type = "lib"] @@ -53,7 +51,7 @@ pub fn naked_with_args_and_return(a: isize) -> isize { #[naked] pub fn naked_recursive() { // CHECK-NEXT: {{.+}}: - // CHECK-NEXT: call void @naked_empty() + // CHECK: call void @naked_empty() // FIXME(#39685) Avoid one block per call. // CHECK-NEXT: br label %bb1 @@ -61,19 +59,19 @@ pub fn naked_recursive() { naked_empty(); - // CHECK-NEXT: %{{[0-9]+}} = call i{{[0-9]+}} @naked_with_return() + // CHECK: %{{[0-9]+}} = call i{{[0-9]+}} @naked_with_return() // FIXME(#39685) Avoid one block per call. // CHECK-NEXT: br label %bb2 // CHECK: bb2: - // CHECK-NEXT: %{{[0-9]+}} = call i{{[0-9]+}} @naked_with_args_and_return(i{{[0-9]+}} %{{[0-9]+}}) + // CHECK: %{{[0-9]+}} = call i{{[0-9]+}} @naked_with_args_and_return(i{{[0-9]+}} %{{[0-9]+}}) // FIXME(#39685) Avoid one block per call. // CHECK-NEXT: br label %bb3 // CHECK: bb3: - // CHECK-NEXT: call void @naked_with_args(i{{[0-9]+}} %{{[0-9]+}}) + // CHECK: call void @naked_with_args(i{{[0-9]+}} %{{[0-9]+}}) // FIXME(#39685) Avoid one block per call. // CHECK-NEXT: br label %bb4 diff --git a/src/test/codegen/non-terminate/infinite-loop-1.rs b/src/test/codegen/non-terminate/infinite-loop-1.rs new file mode 100644 index 0000000000000..fa9c66b47c0a4 --- /dev/null +++ b/src/test/codegen/non-terminate/infinite-loop-1.rs @@ -0,0 +1,17 @@ +// compile-flags: -C opt-level=3 + +#![crate_type = "lib"] + +fn infinite_loop() -> u8 { + loop {} +} + +// CHECK-LABEL: @test +#[no_mangle] +fn test() -> u8 { + // CHECK-NOT: unreachable + // CHECK: br label %{{.+}} + // CHECK-NOT: unreachable + let x = infinite_loop(); + x +} diff --git a/src/test/codegen/non-terminate/infinite-loop-2.rs b/src/test/codegen/non-terminate/infinite-loop-2.rs new file mode 100644 index 0000000000000..81d62ab33d778 --- /dev/null +++ b/src/test/codegen/non-terminate/infinite-loop-2.rs @@ -0,0 +1,19 @@ +// compile-flags: -C opt-level=3 + +#![crate_type = "lib"] + +fn infinite_loop() -> u8 { + let i = 2; + while i > 1 {} + 1 +} + +// CHECK-LABEL: @test +#[no_mangle] +fn test() -> u8 { + // CHECK-NOT: unreachable + // CHECK: br label %{{.+}} + // CHECK-NOT: unreachable + let x = infinite_loop(); + x +} diff --git a/src/test/codegen/non-terminate/infinite-recursion.rs b/src/test/codegen/non-terminate/infinite-recursion.rs new file mode 100644 index 0000000000000..6d1f2d4bf8f4a --- /dev/null +++ b/src/test/codegen/non-terminate/infinite-recursion.rs @@ -0,0 +1,14 @@ +// compile-flags: -C opt-level=3 + +#![crate_type = "lib"] + +#![allow(unconditional_recursion)] + +// CHECK-LABEL: @infinite_recursion +#[no_mangle] +fn infinite_recursion() -> u8 { + // CHECK-NOT: ret i8 undef + // CHECK: br label %{{.+}} + // CHECK-NOT: ret i8 undef + infinite_recursion() +} diff --git a/src/test/codegen/repeat-trusted-len.rs b/src/test/codegen/repeat-trusted-len.rs index 87f29f6047c6a..40399e8f76f01 100644 --- a/src/test/codegen/repeat-trusted-len.rs +++ b/src/test/codegen/repeat-trusted-len.rs @@ -14,6 +14,10 @@ pub fn helper(_: usize) { // CHECK-LABEL: @repeat_take_collect #[no_mangle] pub fn repeat_take_collect() -> Vec { -// CHECK: call void @llvm.memset.p0i8.[[USIZE]](i8* {{(nonnull )?}}align 1 %{{[0-9]+}}, i8 42, [[USIZE]] 100000, i1 false) +// FIXME: At the time of writing LLVM transforms this loop into a single +// `store` and then a `memset` with size = 99999. The correct check should be: +// call void @llvm.memset.p0i8.[[USIZE]](i8* {{(nonnull )?}}align 1 %{{[a-z0-9.]+}}, i8 42, [[USIZE]] 100000, i1 false) + +// CHECK: call void @llvm.memset.p0i8.[[USIZE]](i8* {{(nonnull )?}}align 1 %{{[a-z0-9.]+}}, i8 42, [[USIZE]] 99999, i1 false) iter::repeat(42).take(100000).collect() } diff --git a/src/test/codegen/vec-clear.rs b/src/test/codegen/vec-clear.rs index b9ffce8b0cb3d..22e1248907a76 100644 --- a/src/test/codegen/vec-clear.rs +++ b/src/test/codegen/vec-clear.rs @@ -1,3 +1,9 @@ +// ignore-test + +// FIXME: +// LLVM can't optimize some loops with unknown number of iterations because of +// @llvm.sideeffect() (see also #59546) + // ignore-debug: the debug assertions get in the way // compile-flags: -O diff --git a/src/test/codegen/vec-iter-collect-len.rs b/src/test/codegen/vec-iter-collect-len.rs index 73348ddd063dc..893d1b50a4a97 100644 --- a/src/test/codegen/vec-iter-collect-len.rs +++ b/src/test/codegen/vec-iter-collect-len.rs @@ -5,8 +5,6 @@ #[no_mangle] pub fn get_len() -> usize { - // CHECK-LABEL: @get_len - // CHECK-NOT: call - // CHECK-NOT: invoke + // CHECK-COUNT-1: {{^define}} [1, 2, 3].iter().collect::>().len() } diff --git a/src/test/codegen/vec-optimizes-away.rs b/src/test/codegen/vec-optimizes-away.rs index ebede0908c6c4..08d0332651cb7 100644 --- a/src/test/codegen/vec-optimizes-away.rs +++ b/src/test/codegen/vec-optimizes-away.rs @@ -8,6 +8,6 @@ pub fn sum_me() -> i32 { // CHECK-LABEL: @sum_me // CHECK-NEXT: {{^.*:$}} - // CHECK-NEXT: ret i32 6 + // CHECK: ret i32 6 vec![1, 2, 3].iter().sum::() } diff --git a/src/test/run-make-fulldeps/inline-always-many-cgu/Makefile b/src/test/run-make-fulldeps/inline-always-many-cgu/Makefile index 0cab955f6442b..fd09df78ffa8d 100644 --- a/src/test/run-make-fulldeps/inline-always-many-cgu/Makefile +++ b/src/test/run-make-fulldeps/inline-always-many-cgu/Makefile @@ -2,7 +2,7 @@ all: $(RUSTC) foo.rs --emit llvm-ir -C codegen-units=2 - if cat $(TMPDIR)/*.ll | $(CGREP) -e '\bcall\b'; then \ + if cat $(TMPDIR)/*.ll | grep -v 'call void @llvm.sideeffect()' | $(CGREP) -e '\bcall\b'; then \ echo "found call instruction when one wasn't expected"; \ exit 1; \ fi From 10c668190cb419b539a36214237382c6689f7daf Mon Sep 17 00:00:00 2001 From: Xiang Fan Date: Sat, 28 Sep 2019 07:13:53 +0800 Subject: [PATCH 2/3] Gate llvm.sideeffect under -Z insert-sideeffect --- src/librustc/session/config.rs | 3 +++ src/librustc_codegen_llvm/intrinsic.rs | 6 ++++-- src/librustc_codegen_ssa/mir/block.rs | 16 +++++++++------- src/test/codegen/alloc-optimisation.rs | 3 +-- src/test/codegen/dealloc-no-unwind.rs | 2 +- src/test/codegen/issue-34947-pow-i32.rs | 9 +++------ src/test/codegen/issue-45222.rs | 6 ------ src/test/codegen/naked-functions.rs | 10 ++++++---- .../codegen/non-terminate/infinite-loop-1.rs | 2 +- .../codegen/non-terminate/infinite-loop-2.rs | 2 +- .../codegen/non-terminate/infinite-recursion.rs | 2 +- src/test/codegen/repeat-trusted-len.rs | 6 +----- src/test/codegen/vec-clear.rs | 6 ------ src/test/codegen/vec-iter-collect-len.rs | 4 +++- src/test/codegen/vec-optimizes-away.rs | 2 +- .../inline-always-many-cgu/Makefile | 2 +- 16 files changed, 36 insertions(+), 45 deletions(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 73b731b07619d..abfbba53b5936 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -1472,6 +1472,9 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, "which mangling version to use for symbol names"), binary_dep_depinfo: bool = (false, parse_bool, [TRACKED], "include artifacts (sysroot, crate dependencies) used during compilation in dep-info"), + insert_sideeffect: bool = (false, parse_bool, [TRACKED], + "fix undefined behavior when a thread doesn't eventually make progress \ + (such as entering an empty infinite loop) by inserting llvm.sideeffect"), } pub fn default_lib_output() -> CrateType { diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index a8734d338df17..e4ac176576053 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -726,8 +726,10 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { } fn sideeffect(&mut self) { - let fnname = self.get_intrinsic(&("llvm.sideeffect")); - self.call(fnname, &[], None); + if self.tcx.sess.opts.debugging_opts.insert_sideeffect { + let fnname = self.get_intrinsic(&("llvm.sideeffect")); + self.call(fnname, &[], None); + } } fn va_start(&mut self, va_list: &'ll Value) -> &'ll Value { diff --git a/src/librustc_codegen_ssa/mir/block.rs b/src/librustc_codegen_ssa/mir/block.rs index 99d70e6bb8253..8e3243f5e7bd0 100644 --- a/src/librustc_codegen_ssa/mir/block.rs +++ b/src/librustc_codegen_ssa/mir/block.rs @@ -157,13 +157,15 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'a, 'tcx> { bx: &mut Bx, targets: &[mir::BasicBlock], ) { - if targets.iter().any(|target| { - *target <= *self.bb - && target - .start_location() - .is_predecessor_of(self.bb.start_location(), mir) - }) { - bx.sideeffect(); + if bx.tcx().sess.opts.debugging_opts.insert_sideeffect { + if targets.iter().any(|target| { + *target <= *self.bb + && target + .start_location() + .is_predecessor_of(self.bb.start_location(), mir) + }) { + bx.sideeffect(); + } } } } diff --git a/src/test/codegen/alloc-optimisation.rs b/src/test/codegen/alloc-optimisation.rs index c1e36a17ec291..c3ffaeb9547b3 100644 --- a/src/test/codegen/alloc-optimisation.rs +++ b/src/test/codegen/alloc-optimisation.rs @@ -7,8 +7,7 @@ pub fn alloc_test(data: u32) { // CHECK-LABEL: @alloc_test // CHECK-NEXT: start: - // CHECK-NOT: alloc - // CHECK: ret void + // CHECK-NEXT: ret void let x = Box::new(data); drop(x); } diff --git a/src/test/codegen/dealloc-no-unwind.rs b/src/test/codegen/dealloc-no-unwind.rs index f8fc663169d9b..ff21b4caa83c3 100644 --- a/src/test/codegen/dealloc-no-unwind.rs +++ b/src/test/codegen/dealloc-no-unwind.rs @@ -17,7 +17,7 @@ impl Drop for A { pub fn a(a: Box) { // CHECK-LABEL: define void @a // CHECK: call void @__rust_dealloc - // CHECK: call void @foo + // CHECK-NEXT: call void @foo let _a = A; drop(a); } diff --git a/src/test/codegen/issue-34947-pow-i32.rs b/src/test/codegen/issue-34947-pow-i32.rs index bd8b79adf43b6..653da8e8b5f7b 100644 --- a/src/test/codegen/issue-34947-pow-i32.rs +++ b/src/test/codegen/issue-34947-pow-i32.rs @@ -6,11 +6,8 @@ #[no_mangle] pub fn issue_34947(x: i32) -> i32 { // CHECK: mul - // CHECK-NOT: br label - // CHECK: mul - // CHECK-NOT: br label - // CHECK: mul - // CHECK-NOT: br label - // CHECK: ret + // CHECK-NEXT: mul + // CHECK-NEXT: mul + // CHECK-NEXT: ret x.pow(5) } diff --git a/src/test/codegen/issue-45222.rs b/src/test/codegen/issue-45222.rs index 894927c5c3351..7aadc8a095498 100644 --- a/src/test/codegen/issue-45222.rs +++ b/src/test/codegen/issue-45222.rs @@ -1,9 +1,3 @@ -// ignore-test - -// FIXME: -// LLVM can't optimize some loops with a large number of iterations because of -// @llvm.sideeffect() (see also #59546) - // compile-flags: -O // ignore-debug: the debug assertions get in the way diff --git a/src/test/codegen/naked-functions.rs b/src/test/codegen/naked-functions.rs index fb683a49a8a80..2050193b61b54 100644 --- a/src/test/codegen/naked-functions.rs +++ b/src/test/codegen/naked-functions.rs @@ -1,3 +1,5 @@ +// ignore-tidy-linelength + // compile-flags: -C no-prepopulate-passes #![crate_type = "lib"] @@ -51,7 +53,7 @@ pub fn naked_with_args_and_return(a: isize) -> isize { #[naked] pub fn naked_recursive() { // CHECK-NEXT: {{.+}}: - // CHECK: call void @naked_empty() + // CHECK-NEXT: call void @naked_empty() // FIXME(#39685) Avoid one block per call. // CHECK-NEXT: br label %bb1 @@ -59,19 +61,19 @@ pub fn naked_recursive() { naked_empty(); - // CHECK: %{{[0-9]+}} = call i{{[0-9]+}} @naked_with_return() + // CHECK-NEXT: %{{[0-9]+}} = call i{{[0-9]+}} @naked_with_return() // FIXME(#39685) Avoid one block per call. // CHECK-NEXT: br label %bb2 // CHECK: bb2: - // CHECK: %{{[0-9]+}} = call i{{[0-9]+}} @naked_with_args_and_return(i{{[0-9]+}} %{{[0-9]+}}) + // CHECK-NEXT: %{{[0-9]+}} = call i{{[0-9]+}} @naked_with_args_and_return(i{{[0-9]+}} %{{[0-9]+}}) // FIXME(#39685) Avoid one block per call. // CHECK-NEXT: br label %bb3 // CHECK: bb3: - // CHECK: call void @naked_with_args(i{{[0-9]+}} %{{[0-9]+}}) + // CHECK-NEXT: call void @naked_with_args(i{{[0-9]+}} %{{[0-9]+}}) // FIXME(#39685) Avoid one block per call. // CHECK-NEXT: br label %bb4 diff --git a/src/test/codegen/non-terminate/infinite-loop-1.rs b/src/test/codegen/non-terminate/infinite-loop-1.rs index fa9c66b47c0a4..56b360e0a7f48 100644 --- a/src/test/codegen/non-terminate/infinite-loop-1.rs +++ b/src/test/codegen/non-terminate/infinite-loop-1.rs @@ -1,4 +1,4 @@ -// compile-flags: -C opt-level=3 +// compile-flags: -C opt-level=3 -Z insert-sideeffect #![crate_type = "lib"] diff --git a/src/test/codegen/non-terminate/infinite-loop-2.rs b/src/test/codegen/non-terminate/infinite-loop-2.rs index 81d62ab33d778..2921ab6dc04af 100644 --- a/src/test/codegen/non-terminate/infinite-loop-2.rs +++ b/src/test/codegen/non-terminate/infinite-loop-2.rs @@ -1,4 +1,4 @@ -// compile-flags: -C opt-level=3 +// compile-flags: -C opt-level=3 -Z insert-sideeffect #![crate_type = "lib"] diff --git a/src/test/codegen/non-terminate/infinite-recursion.rs b/src/test/codegen/non-terminate/infinite-recursion.rs index 6d1f2d4bf8f4a..1f292ce379fc0 100644 --- a/src/test/codegen/non-terminate/infinite-recursion.rs +++ b/src/test/codegen/non-terminate/infinite-recursion.rs @@ -1,4 +1,4 @@ -// compile-flags: -C opt-level=3 +// compile-flags: -C opt-level=3 -Z insert-sideeffect #![crate_type = "lib"] diff --git a/src/test/codegen/repeat-trusted-len.rs b/src/test/codegen/repeat-trusted-len.rs index 40399e8f76f01..87f29f6047c6a 100644 --- a/src/test/codegen/repeat-trusted-len.rs +++ b/src/test/codegen/repeat-trusted-len.rs @@ -14,10 +14,6 @@ pub fn helper(_: usize) { // CHECK-LABEL: @repeat_take_collect #[no_mangle] pub fn repeat_take_collect() -> Vec { -// FIXME: At the time of writing LLVM transforms this loop into a single -// `store` and then a `memset` with size = 99999. The correct check should be: -// call void @llvm.memset.p0i8.[[USIZE]](i8* {{(nonnull )?}}align 1 %{{[a-z0-9.]+}}, i8 42, [[USIZE]] 100000, i1 false) - -// CHECK: call void @llvm.memset.p0i8.[[USIZE]](i8* {{(nonnull )?}}align 1 %{{[a-z0-9.]+}}, i8 42, [[USIZE]] 99999, i1 false) +// CHECK: call void @llvm.memset.p0i8.[[USIZE]](i8* {{(nonnull )?}}align 1 %{{[0-9]+}}, i8 42, [[USIZE]] 100000, i1 false) iter::repeat(42).take(100000).collect() } diff --git a/src/test/codegen/vec-clear.rs b/src/test/codegen/vec-clear.rs index 22e1248907a76..b9ffce8b0cb3d 100644 --- a/src/test/codegen/vec-clear.rs +++ b/src/test/codegen/vec-clear.rs @@ -1,9 +1,3 @@ -// ignore-test - -// FIXME: -// LLVM can't optimize some loops with unknown number of iterations because of -// @llvm.sideeffect() (see also #59546) - // ignore-debug: the debug assertions get in the way // compile-flags: -O diff --git a/src/test/codegen/vec-iter-collect-len.rs b/src/test/codegen/vec-iter-collect-len.rs index 893d1b50a4a97..73348ddd063dc 100644 --- a/src/test/codegen/vec-iter-collect-len.rs +++ b/src/test/codegen/vec-iter-collect-len.rs @@ -5,6 +5,8 @@ #[no_mangle] pub fn get_len() -> usize { - // CHECK-COUNT-1: {{^define}} + // CHECK-LABEL: @get_len + // CHECK-NOT: call + // CHECK-NOT: invoke [1, 2, 3].iter().collect::>().len() } diff --git a/src/test/codegen/vec-optimizes-away.rs b/src/test/codegen/vec-optimizes-away.rs index 08d0332651cb7..ebede0908c6c4 100644 --- a/src/test/codegen/vec-optimizes-away.rs +++ b/src/test/codegen/vec-optimizes-away.rs @@ -8,6 +8,6 @@ pub fn sum_me() -> i32 { // CHECK-LABEL: @sum_me // CHECK-NEXT: {{^.*:$}} - // CHECK: ret i32 6 + // CHECK-NEXT: ret i32 6 vec![1, 2, 3].iter().sum::() } diff --git a/src/test/run-make-fulldeps/inline-always-many-cgu/Makefile b/src/test/run-make-fulldeps/inline-always-many-cgu/Makefile index fd09df78ffa8d..0cab955f6442b 100644 --- a/src/test/run-make-fulldeps/inline-always-many-cgu/Makefile +++ b/src/test/run-make-fulldeps/inline-always-many-cgu/Makefile @@ -2,7 +2,7 @@ all: $(RUSTC) foo.rs --emit llvm-ir -C codegen-units=2 - if cat $(TMPDIR)/*.ll | grep -v 'call void @llvm.sideeffect()' | $(CGREP) -e '\bcall\b'; then \ + if cat $(TMPDIR)/*.ll | $(CGREP) -e '\bcall\b'; then \ echo "found call instruction when one wasn't expected"; \ exit 1; \ fi From e9acfa306f47a40be27e8cf72c55dbec35d94017 Mon Sep 17 00:00:00 2001 From: Xiang Fan Date: Sat, 28 Sep 2019 07:14:21 +0800 Subject: [PATCH 3/3] Generate llvm.sideeffect at function entry instead of call --- src/librustc_codegen_llvm/intrinsic.rs | 4 +++- src/librustc_codegen_ssa/mir/block.rs | 11 +++++++---- src/librustc_codegen_ssa/mir/mod.rs | 2 ++ src/librustc_codegen_ssa/mir/rvalue.rs | 1 - 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index e4ac176576053..68d9af09c42b1 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -124,7 +124,6 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { self.call(expect, &[args[0].immediate(), self.const_bool(false)], None) } "try" => { - self.sideeffect(); try_intrinsic(self, args[0].immediate(), args[1].immediate(), @@ -818,6 +817,7 @@ fn codegen_msvc_try( ) { let llfn = get_rust_try_fn(bx, &mut |mut bx| { bx.set_personality_fn(bx.eh_personality()); + bx.sideeffect(); let mut normal = bx.build_sibling_block("normal"); let mut catchswitch = bx.build_sibling_block("catchswitch"); @@ -941,6 +941,8 @@ fn codegen_gnu_try( // expected to be `*mut *mut u8` for this to actually work, but that's // managed by the standard library. + bx.sideeffect(); + let mut then = bx.build_sibling_block("then"); let mut catch = bx.build_sibling_block("catch"); diff --git a/src/librustc_codegen_ssa/mir/block.rs b/src/librustc_codegen_ssa/mir/block.rs index 8e3243f5e7bd0..d95c0b2d87b4b 100644 --- a/src/librustc_codegen_ssa/mir/block.rs +++ b/src/librustc_codegen_ssa/mir/block.rs @@ -364,7 +364,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { FnType::of_instance(&bx, drop_fn)) } }; - bx.sideeffect(); + helper.maybe_sideeffect(self.mir, &mut bx, &[target]); helper.do_call(self, &mut bx, fn_ty, drop_fn, args, Some((ReturnDest::Nothing, target)), unwind); @@ -464,7 +464,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let fn_ty = FnType::of_instance(&bx, instance); let llfn = bx.get_fn(instance); - bx.sideeffect(); // Codegen the actual panic invoke/call. helper.do_call(self, &mut bx, fn_ty, llfn, &args, None, cleanup); } @@ -584,7 +583,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let fn_ty = FnType::of_instance(&bx, instance); let llfn = bx.get_fn(instance); - bx.sideeffect(); + if let Some((_, target)) = destination.as_ref() { + helper.maybe_sideeffect(self.mir, &mut bx, &[*target]); + } // Codegen the actual panic invoke/call. helper.do_call( self, @@ -820,7 +821,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { _ => span_bug!(span, "no llfn for call"), }; - bx.sideeffect(); + if let Some((_, target)) = destination.as_ref() { + helper.maybe_sideeffect(self.mir, &mut bx, &[*target]); + } helper.do_call(self, &mut bx, fn_ty, fn_ptr, &llargs, destination.as_ref().map(|&(_, target)| (ret_dest, target)), cleanup); diff --git a/src/librustc_codegen_ssa/mir/mod.rs b/src/librustc_codegen_ssa/mir/mod.rs index c341c0685c6a7..e4fdf0a09bbfc 100644 --- a/src/librustc_codegen_ssa/mir/mod.rs +++ b/src/librustc_codegen_ssa/mir/mod.rs @@ -204,6 +204,8 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( bx.set_personality_fn(cx.eh_personality()); } + bx.sideeffect(); + let cleanup_kinds = analyze::cleanup_kinds(&mir); // Allocate a `Block` for every basic block, except // the start block, if nothing loops back to it. diff --git a/src/librustc_codegen_ssa/mir/rvalue.rs b/src/librustc_codegen_ssa/mir/rvalue.rs index 11a75c6f38c7f..9b69383b455cf 100644 --- a/src/librustc_codegen_ssa/mir/rvalue.rs +++ b/src/librustc_codegen_ssa/mir/rvalue.rs @@ -486,7 +486,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { }; let instance = ty::Instance::mono(bx.tcx(), def_id); let r = bx.cx().get_fn(instance); - bx.sideeffect(); let call = bx.call(r, &[llsize, llalign], None); let val = bx.pointercast(call, llty_ptr);