[llvm] [CodeGen] Correctly handle non-standard cases in RemoveLoadsIntoFakeUses (PR #111551)

[Stephen Tozer via llvm-commits]

================
@@ -109,31 +111,38 @@ bool RemoveLoadsIntoFakeUses::runOnMachineFunction(MachineFunction &MF) {
       // reload of a spilled register.
       if (MI.getRestoreSize(TII)) {
         Register Reg = MI.getOperand(0).getReg();
-        assert(Reg.isPhysical() && "VReg seen in function with NoVRegs set?");
         // Don't delete live physreg defs, or any reserved register defs.
         if (!LivePhysRegs.available(Reg) || MRI->isReserved(Reg))
           continue;
-        // There should be an exact match between the loaded register and the
-        // FAKE_USE use. If not, this is a load that is unused by anything? It
-        // should probably be deleted, but that's outside of this pass' scope.
-        if (RegFakeUses.contains(Reg)) {
+        // There should typically be an exact match between the loaded register
+        // and the FAKE_USE, but sometimes regalloc will choose to load a larger
+        // value than is needed. Therefore, as long as the load isn't used by
+        // anything except at least one FAKE_USE, we will delete it. If it isn't
+        // used by any fake uses, it should still be safe to delete but we
+        // choose to ignore it so that this pass has no side effects unrelated
+        // to fake uses.
+        SmallDenseSet<MachineInstr *> FakeUsesToDelete;
+        SmallVector<MachineInstr *> RemainingFakeUses;
+        for (MachineInstr *&FakeUse : reverse(RegFakeUses)) {
+          if (FakeUse->readsRegister(Reg, TRI)) {
+            FakeUsesToDelete.insert(FakeUse);
+            RegFakeUses.erase(&FakeUse);
+          }
+        }
+        if (!FakeUsesToDelete.empty()) {
           LLVM_DEBUG(dbgs() << "RemoveLoadsIntoFakeUses: DELETING: " << MI);
-          // It is possible that some DBG_VALUE instructions refer to this
-          // instruction. They will be deleted in the live debug variable
-          // analysis.
+          // Since this load only exists to restore a spilled register and we
+          // haven't, run LiveDebugValues yet, there shouldn't be any DBG_VALUEs
+          // for this load; otherwise, deleting this would be incorrect.
----------------
SLTozer wrote:

I think the easiest way would be to disable this specific pass for non-instruction-referencing builds - it's a relatively small optimization, and not necessary for correctness. The alternative would be to track uses by DBG_VALUES to check that the loaded value isn't used by one; any preferences?

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