Remove the -Zinsert-sideeffect

This removes all of the code we had in place to work-around LLVM's
handling of forward progress. From this removal excluded is a workaround
where we'd insert a `sideeffect` into clearly infinite loops such as
`loop {}`. This code remains conditionally effective when the LLVM
version is earlier than 12.0, which fixed the forward progress related
miscompilations at their root.

3 files changed, 12 insertions, 55 deletions

diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs
index e148ed7ad3b..a8dda100763 100644
--- a/compiler/rustc_codegen_ssa/src/mir/block.rs
+++ b/compiler/rustc_codegen_ssa/src/mir/block.rs
@@ -146,24 +146,6 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> {
             }
         }
     }
-
-    // Generate sideeffect intrinsic if jumping to any of the targets can form
-    // a loop.
-    fn maybe_sideeffect<Bx: BuilderMethods<'a, 'tcx>>(
-        &self,
-        mir: &'tcx mir::Body<'tcx>,
-        bx: &mut Bx,
-        targets: &[mir::BasicBlock],
-    ) {
-        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(false);
-            }
-        }
-    }
 }
 
 /// Codegen implementations for some terminator variants.
@@ -198,8 +180,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
         let discr = self.codegen_operand(&mut bx, &discr);
         // `switch_ty` is redundant, sanity-check that.
         assert_eq!(discr.layout.ty, switch_ty);
-        helper.maybe_sideeffect(self.mir, &mut bx, targets.all_targets());
-
         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
@@ -308,7 +288,6 @@ 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;
         }
@@ -337,7 +316,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
             }
             _ => (bx.get_fn_addr(drop_fn), FnAbi::of_instance(&bx, drop_fn, &[])),
         };
-        helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
         helper.do_call(
             self,
             &mut bx,
@@ -379,7 +357,6 @@ 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;
         }
@@ -390,7 +367,6 @@ 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 {
@@ -491,9 +467,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                 let fn_abi = FnAbi::of_instance(bx, instance, &[]);
                 let llfn = bx.get_fn_addr(instance);
 
-                if let Some((_, target)) = destination.as_ref() {
-                    helper.maybe_sideeffect(self.mir, bx, &[*target]);
-                }
                 // Codegen the actual panic invoke/call.
                 helper.do_call(
                     self,
@@ -507,7 +480,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
             } else {
                 // a NOP
                 let target = destination.as_ref().unwrap().1;
-                helper.maybe_sideeffect(self.mir, bx, &[target]);
                 helper.funclet_br(self, bx, target)
             }
             true
@@ -551,7 +523,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
         if let Some(ty::InstanceDef::DropGlue(_, None)) = def {
             // 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;
         }
@@ -586,7 +557,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
             if let Some(destination_ref) = destination.as_ref() {
                 let &(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,
@@ -634,8 +604,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                     location.val.store(&mut bx, tmp);
                 }
                 self.store_return(&mut bx, ret_dest, &fn_abi.ret, location.immediate());
-
-                helper.maybe_sideeffect(self.mir, &mut bx, &[*target]);
                 helper.funclet_br(self, &mut bx, *target);
             }
             return;
@@ -700,7 +668,6 @@ 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();
@@ -817,9 +784,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
             _ => span_bug!(span, "no llfn for call"),
         };
 
-        if let Some((_, target)) = destination.as_ref() {
-            helper.maybe_sideeffect(self.mir, &mut bx, &[*target]);
-        }
         helper.do_call(
             self,
             &mut bx,
@@ -969,22 +933,16 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
 
             mir::TerminatorKind::Goto { target } => {
                 if bb == target {
-                    // This is an unconditional branch back to this same basic
-                    // block. That means we have something like a `loop {}`
-                    // statement. Currently LLVM miscompiles this because it
-                    // assumes forward progress. We want to prevent this in all
-                    // cases, but that has a fairly high cost to compile times
-                    // currently. Instead, try to handle this specific case
-                    // which comes up commonly in practice (e.g., in embedded
-                    // code).
+                    // This is an unconditional branch back to this same basic block. That means we
+                    // have something like a `loop {}` statement. LLVM versions before 12.0
+                    // miscompile this because they assume forward progress. For older versions
+                    // try to handle just this specific case which comes up commonly in practice
+                    // (e.g., in embedded code).
                     //
-                    // The `true` here means we insert side effects regardless
-                    // of -Zinsert-sideeffect being passed on unconditional
-                    // branching to the same basic block.
-                    bx.sideeffect(true);
-                } else {
-                    helper.maybe_sideeffect(self.mir, &mut bx, &[target]);
+                    // NB: the `sideeffect` currently checks for the LLVM version used internally.
+                    bx.sideeffect();
                 }
+
                 helper.funclet_br(self, &mut bx, target);
             }
 
diff --git a/compiler/rustc_codegen_ssa/src/mir/mod.rs b/compiler/rustc_codegen_ssa/src/mir/mod.rs
index d31ececf130..3f945478213 100644
--- a/compiler/rustc_codegen_ssa/src/mir/mod.rs
+++ b/compiler/rustc_codegen_ssa/src/mir/mod.rs
@@ -149,8 +149,6 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
         bx.set_personality_fn(cx.eh_personality());
     }
 
-    bx.sideeffect(false);
-
     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/compiler/rustc_codegen_ssa/src/traits/intrinsic.rs b/compiler/rustc_codegen_ssa/src/traits/intrinsic.rs
index ac3c99f9c90..777436ad2ae 100644
--- a/compiler/rustc_codegen_ssa/src/traits/intrinsic.rs
+++ b/compiler/rustc_codegen_ssa/src/traits/intrinsic.rs
@@ -20,9 +20,10 @@ 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;
-    /// Normally, sideeffect is only emitted if -Zinsert-sideeffect is passed;
-    /// in some cases though we want to emit it regardless.
-    fn sideeffect(&mut self, unconditional: bool);
+    /// Emits a forced side effect.
+    ///
+    /// Currently has any effect only when LLVM versions prior to 12.0 are used as the backend.
+    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;