[CodeGen] Check physical def kill flag in MachineInstr::isDead

[lukel97]

Currently when checking if a MachineInstr is dead we use LiveRegUnits to check if any physical defs are alive. However if the kill/dead flag is set on the def, then we don't need to rely on LiveRegUnits check.

This should save some cycles on the hot path and allows users of isDead which don't pass in LiveRegUnits to catch more dead instructions.

[llvmbot]

@llvm/pr-subscribers-backend-risc-v

Author: Luke Lau (lukel97)

Changes

Currently when checking if a MachineInstr is dead we use LiveRegUnits to check if any physical defs are alive. However if the kill/dead flag is set on the def, then we don't need to rely on LiveRegUnits check.

This should save some cycles on the hot path and allows users of isDead which don't pass in LiveRegUnits to catch more dead instructions.

---

Full diff: https://github.com/llvm/llvm-project/pull/168684.diff

2 Files Affected:

• (modified) llvm/lib/CodeGen/MachineInstr.cpp (+2-1)
• (added) llvm/test/CodeGen/RISCV/machine-sink.mir (+85)

diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp
index eb46124d9eb5f..d6790714b9660 100644
--- a/llvm/lib/CodeGen/MachineInstr.cpp
+++ b/llvm/lib/CodeGen/MachineInstr.cpp
@@ -1398,7 +1398,8 @@ bool MachineInstr::isDead(const MachineRegisterInfo &MRI,
     Register Reg = MO.getReg();
     if (Reg.isPhysical()) {
       // Don't delete live physreg defs, or any reserved register defs.
-      if (!LivePhysRegs || !LivePhysRegs->available(Reg) || MRI.isReserved(Reg))
+      if ((!MO.isDead() && (!LivePhysRegs || !LivePhysRegs->available(Reg))) ||
+          MRI.isReserved(Reg))
         return false;
     } else {
       if (MO.isDead())
diff --git a/llvm/test/CodeGen/RISCV/machine-sink.mir b/llvm/test/CodeGen/RISCV/machine-sink.mir
new file mode 100644
index 0000000000000..213e6fd73e196
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/machine-sink.mir
@@ -0,0 +1,85 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 6
+# RUN: llc -mtriple=riscv64 -run-pass machine-sink -sink-insts-to-avoid-spills -o - %s | FileCheck %s
+---
+name: no_delete_live_phys_def
+tracksRegLiveness: true
+body: |
+  ; CHECK-LABEL: name: no_delete_live_phys_def
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT:   liveins: $x8
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   %x:gpr = COPY $x8
+  ; CHECK-NEXT:   %y:gpr = COPY %x, implicit-def $x8
+  ; CHECK-NEXT:   PseudoBR %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   %z:gpr = ADDI %y, 1
+  ; CHECK-NEXT:   PseudoBR %bb.1
+  bb.0:
+    liveins: $x8
+    %x:gpr = COPY $x8
+    %y:gpr = COPY %x, implicit-def $x8
+    PseudoBR %bb.1
+  bb.1:
+    %z:gpr = ADDI %y, 1
+    PseudoBR %bb.1
+...
+---
+name: delete_dead_phys_def
+tracksRegLiveness: true
+body: |
+  ; CHECK-LABEL: name: delete_dead_phys_def
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT:   liveins: $x8
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   %x:gpr = COPY $x8
+  ; CHECK-NEXT:   PseudoBR %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gpr = COPY %x, implicit-def dead $x8
+  ; CHECK-NEXT:   %z:gpr = ADDI [[COPY]], 1
+  ; CHECK-NEXT:   PseudoBR %bb.1
+  bb.0:
+    liveins: $x8
+    %x:gpr = COPY $x8
+    %y:gpr = COPY %x, implicit-def dead $x8
+    PseudoBR %bb.1
+  bb.1:
+    %z:gpr = ADDI %y, 1
+    PseudoBR %bb.1
+...
+---
+name: reserved_phys_def
+tracksRegLiveness: true
+body: |
+  ; CHECK-LABEL: name: reserved_phys_def
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT:   liveins: $x8
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   %x:gpr = COPY $x8
+  ; CHECK-NEXT:   %y:gpr = COPY %x, implicit-def dead $x2
+  ; CHECK-NEXT:   PseudoBR %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gpr = COPY %x, implicit-def dead $x2
+  ; CHECK-NEXT:   %z:gpr = ADDI [[COPY]], 1
+  ; CHECK-NEXT:   PseudoBR %bb.1
+  bb.0:
+    liveins: $x8
+    %x:gpr = COPY $x8
+    %y:gpr = COPY %x, implicit-def dead $x2
+    PseudoBR %bb.1
+
+  bb.1:
+    %z:gpr = ADDI %y, 1
+    PseudoBR %bb.1
+...

[s-barannikov]

Is this flag only reliable when MRI.tracksLiveness() is true?

[lukel97]

Thanks for catching that, fixed in 577cf9a

[github-actions]

🐧 Linux x64 Test Results

• 186355 tests passed
• 4859 tests skipped