[PATCH] D78161: [MachineDCE] Predicated instructions shouldn't clear LivePhysRegs.

[Marcello Maggioni via Phabricator via llvm-commits]

kariddi created this revision.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

Predicated instructions might not endup being executed. If that happens
they don't really kill the registers they right, so we can't DCE
instructions that are overwritten by potentially predicated
instructions. Fixing MachineDCE to reflect that.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78161

Files:
  llvm/lib/CodeGen/DeadMachineInstructionElim.cpp
  llvm/test/CodeGen/Hexagon/dead-code-elim-predicated-instr.mir


Index: llvm/test/CodeGen/Hexagon/dead-code-elim-predicated-instr.mir
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/Hexagon/dead-code-elim-predicated-instr.mir
@@ -0,0 +1,39 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -march=hexagon -run-pass dead-mi-elimination -verify-machineinstrs %s -o - | FileCheck %s
+
+# DeadMachineCodeElimination considers predicated instructions as always run. This causes the pass to delete
+# as dead instructions writing to the same registers before the predicated instruction.
+# If the instruction though is not ran because of the predication removing such an instruction is wrong
+# and it shouldn't be done.
+# This test makes sure that that is the case.
+
+---
+name: predicated_instruction_not_preserved
+tracksRegLiveness: true
+
+body: |
+  ; CHECK-LABEL: name: predicated_instruction_not_preserved
+  ; CHECK: bb.0:
+  ; CHECK:   successors: %bb.1(0x80000000)
+  ; CHECK:   liveins: $r0
+  ; CHECK: bb.1:
+  ; CHECK:   successors: %bb.1(0x80000000)
+  ; CHECK:   liveins: $r0
+  ; CHECK:   A2_nop implicit-def $p0
+  ; CHECK:   $r0 = C2_cmoveit $p0, 123
+  ; CHECK:   J2_jumpt killed $p0, %bb.1, implicit-def dead $pc
+  ; CHECK:   J2_jumpr $r31, implicit-def dead $pc
+  bb.0:
+    liveins: $r0
+    successors: %bb.1
+
+  bb.1:
+    successors: %bb.1
+    liveins: $r0
+    A2_nop implicit-def $p0
+    $r1 = A2_sxth killed $r0
+    $r0 = A2_tfrsi 1
+    $r0 = C2_cmoveit $p0, 123
+    J2_jumpt killed $p0, %bb.1, implicit-def dead $pc
+    J2_jumpr $r31, implicit-def dead $pc
+...
Index: llvm/lib/CodeGen/DeadMachineInstructionElim.cpp
===================================================================
--- llvm/lib/CodeGen/DeadMachineInstructionElim.cpp
+++ llvm/lib/CodeGen/DeadMachineInstructionElim.cpp
@@ -14,6 +14,7 @@
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/Passes.h"
+#include "llvm/CodeGen/TargetInstrInfo.h"
 #include "llvm/CodeGen/TargetSubtargetInfo.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Pass.h"
@@ -146,22 +147,26 @@
         continue;
       }
 
-      // Record the physreg defs.
-      for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
-        const MachineOperand &MO = MI->getOperand(i);
-        if (MO.isReg() && MO.isDef()) {
-          Register Reg = MO.getReg();
-          if (Register::isPhysicalRegister(Reg)) {
-            // Check the subreg set, not the alias set, because a def
-            // of a super-register may still be partially live after
-            // this def.
-            for (MCSubRegIterator SR(Reg, TRI,/*IncludeSelf=*/true);
-                 SR.isValid(); ++SR)
-              LivePhysRegs.reset(*SR);
+      // Record the physreg defs. If the instruction is potentially predicated
+      // then do not clear its defs as it could be endup not being executed,
+      // so it doesn't really kill the register it writes in all cases.
+      if (TII->isPredicated(*MI)) {
+        for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
+          const MachineOperand &MO = MI->getOperand(i);
+          if (MO.isReg() && MO.isDef()) {
+            Register Reg = MO.getReg();
+            if (Register::isPhysicalRegister(Reg)) {
+              // Check the subreg set, not the alias set, because a def
+              // of a super-register may still be partially live after
+              // this def.
+              for (MCSubRegIterator SR(Reg, TRI, /*IncludeSelf=*/true);
+                   SR.isValid(); ++SR)
+                LivePhysRegs.reset(*SR);
+            }
+          } else if (MO.isRegMask()) {
+            // Register mask of preserved registers. All clobbers are dead.
+            LivePhysRegs.clearBitsNotInMask(MO.getRegMask());
           }
-        } else if (MO.isRegMask()) {
-          // Register mask of preserved registers. All clobbers are dead.
-          LivePhysRegs.clearBitsNotInMask(MO.getRegMask());
         }
       }
       // Record the physreg uses, after the defs, in case a physreg is


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D78161.257521.patch
Type: text/x-patch
Size: 4149 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200414/8ead4ea4/attachment.bin>