[llvm] [AMDGPU] Analyze implicit reg operands when generating swaps (PR #192220)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-amdgpu

Author: LU-JOHN

<details>
<summary>Changes</summary>

Fix register usage analysis during swap generation by including implicit register operands.  Critical change was to replace MachineInstr uses/defs with all_uses/all_defs.

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


2 Files Affected:

- (modified) llvm/lib/Target/AMDGPU/SIShrinkInstructions.cpp (+24-16) 
- (modified) llvm/test/CodeGen/AMDGPU/v_swap_b32.mir (+19) 


``````````diff
diff --git a/llvm/lib/Target/AMDGPU/SIShrinkInstructions.cpp b/llvm/lib/Target/AMDGPU/SIShrinkInstructions.cpp
index 14ed778f44f3a..a0ad3e764cf81 100644
--- a/llvm/lib/Target/AMDGPU/SIShrinkInstructions.cpp
+++ b/llvm/lib/Target/AMDGPU/SIShrinkInstructions.cpp
@@ -48,11 +48,9 @@ class SIShrinkInstructions {
   bool shrinkMadFma(MachineInstr &MI) const;
   ChangeKind shrinkScalarLogicOp(MachineInstr &MI) const;
   bool tryReplaceDeadSDST(MachineInstr &MI) const;
-  bool instAccessReg(iterator_range<MachineInstr::const_mop_iterator> &&R,
-                     Register Reg, unsigned SubReg) const;
-  bool instReadsReg(const MachineInstr *MI, unsigned Reg,
+  bool instReadsReg(const MachineInstr *MI, Register Reg,
                     unsigned SubReg) const;
-  bool instModifiesReg(const MachineInstr *MI, unsigned Reg,
+  bool instModifiesReg(const MachineInstr *MI, Register Reg,
                        unsigned SubReg) const;
   TargetInstrInfo::RegSubRegPair getSubRegForIndex(Register Reg, unsigned Sub,
                                                    unsigned I) const;
@@ -617,12 +615,11 @@ ChangeKind SIShrinkInstructions::shrinkScalarLogicOp(MachineInstr &MI) const {
   return ChangeKind::None;
 }
 
-// This is the same as MachineInstr::readsRegister/modifiesRegister except
+// This is the same as MachineInstr::readsRegister except
 // it takes subregs into account.
-bool SIShrinkInstructions::instAccessReg(
-    iterator_range<MachineInstr::const_mop_iterator> &&R, Register Reg,
-    unsigned SubReg) const {
-  for (const MachineOperand &MO : R) {
+bool SIShrinkInstructions::instReadsReg(const MachineInstr *MI, Register Reg,
+                                        unsigned SubReg) const {
+  for (const MachineOperand &MO : MI->all_uses()) {
     if (!MO.isReg())
       continue;
 
@@ -639,14 +636,25 @@ bool SIShrinkInstructions::instAccessReg(
   return false;
 }
 
-bool SIShrinkInstructions::instReadsReg(const MachineInstr *MI, unsigned Reg,
-                                        unsigned SubReg) const {
-  return instAccessReg(MI->uses(), Reg, SubReg);
-}
-
-bool SIShrinkInstructions::instModifiesReg(const MachineInstr *MI, unsigned Reg,
+// This is the same as MachineInstr::modifiesRegister except
+// it takes subregs into account.
+bool SIShrinkInstructions::instModifiesReg(const MachineInstr *MI, Register Reg,
                                            unsigned SubReg) const {
-  return instAccessReg(MI->defs(), Reg, SubReg);
+  for (const MachineOperand &MO : MI->all_defs()) {
+    if (!MO.isReg())
+      continue;
+
+    if (Reg.isPhysical() && MO.getReg().isPhysical()) {
+      if (TRI->regsOverlap(Reg, MO.getReg()))
+        return true;
+    } else if (MO.getReg() == Reg && Reg.isVirtual()) {
+      LaneBitmask Overlap = TRI->getSubRegIndexLaneMask(SubReg) &
+                            TRI->getSubRegIndexLaneMask(MO.getSubReg());
+      if (Overlap.any())
+        return true;
+    }
+  }
+  return false;
 }
 
 TargetInstrInfo::RegSubRegPair
diff --git a/llvm/test/CodeGen/AMDGPU/v_swap_b32.mir b/llvm/test/CodeGen/AMDGPU/v_swap_b32.mir
index 27229cd518028..7dbcd295ef4aa 100644
--- a/llvm/test/CodeGen/AMDGPU/v_swap_b32.mir
+++ b/llvm/test/CodeGen/AMDGPU/v_swap_b32.mir
@@ -967,3 +967,22 @@ body:             |
     $vgpr1 = V_MOV_B32_e32 killed $vgpr3, implicit $exec, implicit $vgpr2, implicit-def $vgpr0_vgpr1, implicit killed $vgpr3
     S_SETPC_B64_return $sgpr30_sgpr31, implicit $vgpr0, implicit $vgpr1
 ...
+
+# GCN-LABEL: negative_test_implicit_blocks_swap
+# GCN:      $vgpr2 = V_MOV_B32_e32 killed $vgpr0, implicit $exec
+# GCN-NEXT: $sgpr30_sgpr31 = SI_CALL killed renamable $sgpr0_sgpr1, 0, implicit-def $vgpr2
+# GCN-NEXT: $vgpr0 = V_MOV_B32_e32 killed $vgpr1, implicit $exec
+# GCN-NEXT: $vgpr1 = V_MOV_B32_e32 killed $vgpr2, implicit $exec
+# GCN-NEXT: S_SETPC_B64_return $sgpr30_sgpr31, implicit $vgpr0, implicit $vgpr1
+---
+name:            negative_test_implicit_blocks_swap
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $vgpr0, $vgpr1, $sgpr0_sgpr1
+    $vgpr2 = V_MOV_B32_e32 killed $vgpr0, implicit $exec
+    $sgpr30_sgpr31 = SI_CALL killed renamable $sgpr0_sgpr1, 0, implicit-def $vgpr2
+    $vgpr0 = V_MOV_B32_e32 killed $vgpr1, implicit $exec
+    $vgpr1 = V_MOV_B32_e32 killed $vgpr2, implicit $exec
+    S_SETPC_B64_return $sgpr30_sgpr31, implicit $vgpr0, implicit $vgpr1
+...

``````````

</details>


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