[MachineDCE] Predicated instructions shouldn't clear LivePhysRegs.

[kariddi]

Summary

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.

[rampitec]

Typo in commit message: "registers they right,".
Otherwise looks reasonable for me, but need someone from Hexagon BE to review it.

[kparzysz]

This isn't completely correct. Predicated instructions should have implicit use of the register that they are modifying (unless it's undefined). The cmoveit doesn't have it. How did you get this MIR?

[hgreving]

s/kill the registers they right/kill the registers they write/

so we can't DCE instructions that are overwritten by potentially predicated instructions

nit: re-phrase?

[hgreving]

LGTM

[kariddi]

I'm abandoning this for now based on the fact that I have another patch that ports DeadMachineInstrs to LivePhysRegs. LivePhysRegs though doesn't seem to care about predicates as well ... Probably the fix might need to go there

llvm/lib/CodeGen/DeadMachineInstructionElim.cpp

 //===- DeadMachineInstructionElim.cpp - Remove dead machine instructions --===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This is an extremely simple MachineInstr-level dead-code-elimination pass.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/Statistic.h"
 #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"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 
for (MachineBasicBlock::reverse_iterator MII = MBB.rbegin(),
         // instruction.  They get marked as undef and will be deleted
         // in the live debug variable analysis.
         MI->eraseFromParentAndMarkDBGValuesForRemoval();
         AnyChanges = true;
         ++NumDeletes;
         continue;
       }
 
-      // Record the physreg defs.
+      // 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);
+              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());
           }
         }
+      }
       // Record the physreg uses, after the defs, in case a physreg is
       // both defined and used in the same instruction.
       for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
         const MachineOperand &MO = MI->getOperand(i);
         if (MO.isReg() && MO.isUse()) {
           Register Reg = MO.getReg();
           if (Register::isPhysicalRegister(Reg)) {
             for (MCRegAliasIterator AI(Reg, TRI, true); AI.isValid(); ++AI)

llvm/test/CodeGen/Hexagon/dead-code-elim-predicated-instr.mir

+# 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
+...