[llvm] [MachineLICM] Only mark live-ins as non-invariant if defined in loop (PR #191755)

[Yuyang Zhang via llvm-commits]

================
@@ -596,20 +596,37 @@ void MachineLICMImpl::HoistRegionPostRA(MachineLoop *CurLoop) {
   SmallVector<CandidateInfo, 32> Candidates;
   SmallDenseSet<int> StoredFIs;
 
-  // Walk the entire region, count number of defs for each register, and
-  // collect potential LICM candidates.
+  // First pass: collect all register units defined within the loop.
+  BitVector LoopRUDefs(NumRegUnits);
+  for (MachineBasicBlock *BB : CurLoop->getBlocks()) {
+    for (MachineInstr &MI : *BB) {
+      for (const MachineOperand &MO : MI.operands()) {
+        if (!MO.isReg() || !MO.isDef())
+          continue;
+        Register Reg = MO.getReg();
+        if (!Reg)
+          continue;
+        for (MCRegUnit Unit : TRI->regunits(Reg))
+          LoopRUDefs.set(static_cast<unsigned>(Unit));
+      }
+    }
+  }
+
+  // Second pass: walk the entire region, count number of defs for each
+  // register, and collect potential LICM candidates.
   for (MachineBasicBlock *BB : CurLoop->getBlocks()) {
     // If the header of the loop containing this basic block is a landing pad,
     // then don't try to hoist instructions out of this loop.
     const MachineLoop *ML = MLI->getLoopFor(BB);
     if (ML && ML->getHeader()->isEHPad()) continue;
 
-    // Conservatively treat live-in's as an external def.
-    // FIXME: That means a reload that're reused in successor block(s) will not
-    // be LICM'ed.
+    // Only treat live-in registers that are also defined within the loop as
+    // non-invariant. Live-ins that are solely defined outside the loop are
+    // loop-invariant and should not block hoisting.
     for (const auto &LI : BB->liveins()) {
       for (MCRegUnit Unit : TRI->regunits(LI.PhysReg))
-        RUDefs.set(static_cast<unsigned>(Unit));
+        if (LoopRUDefs.test(static_cast<unsigned>(Unit)))
+          RUDefs.set(static_cast<unsigned>(Unit));
----------------
yuyzhang512 wrote:

`RUDefs` is still needed for on-the-fly multi-def detection. It is built incrementally, so when `ProcessMI` reaches a definition of a register unit already present in `RUDefs`, that register unit has been defined by another instruction and is added to `RUClobbers`. `LoopRUDefs` cannot serve this purpose because it already contains the current instruction's own definition, making every definition appear to be a redefinition.

The live-in marking is also required for correctness rather than merely being conservative. Without it, a loop-carried register that is defined only once in the loop would no longer be forced into `RUClobbers`, allowing its definition to be hoisted. For example:
```
loop:
  $x2 = ADDXri $x0, 0, 0    ; reads live-in $x0
  $x0 = MOVi64imm 5         ; single invariant def of $x0
```
Without the live-in marking, `$x0 = MOVi64imm 5` is hoisted into the preheader, causing the first iteration to read `5` instead of the incoming value of `$x0`, resulting in a miscompile.

https://github.com/llvm/llvm-project/pull/191755