AMDGPU: Fix infinite searches in SIFixSGPRCopies

Summary:
Two conditions could lead to infinite loops when processing PHI nodes in
SIFixSGPRCopies.

The first condition involves a REG_SEQUENCE that uses registers defined by both
a PHI and a COPY.

The second condition arises when a physical register is copied to a virtual
register which is then used in a PHI node. If the same virtual register is
copied to the same physical register, the result is an endless loop.

%0:sgpr_64 = COPY $sgpr0_sgpr1
%2 = PHI %0, %bb.0, %1, %bb.1
$sgpr0_sgpr1 = COPY %0

Reviewers: alex-t, rampitec, arsenm

Reviewed By: rampitec

Subscribers: kzhuravl, jvesely, wdng, nhaehnle, yaxunl, dstuttard, tpr, t-tye, hiraditya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D68970

llvm-svn: 374944

diff --git a/llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp b/llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp
index 02a0518..1966507 100644 (file)
--- a/llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp
+++ b/llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp
@@ -697,7 +697,9 @@ bool SIFixSGPRCopies::runOnMachineFunction(MachineFunction &MF) {
 void SIFixSGPRCopies::processPHINode(MachineInstr &MI) {
   unsigned numVGPRUses = 0;
   SetVector<const MachineInstr *> worklist;
+  SmallSet<const MachineInstr *, 4> Visited;
   worklist.insert(&MI);
+  Visited.insert(&MI);
   while (!worklist.empty()) {
     const MachineInstr *Instr = worklist.pop_back_val();
     unsigned Reg = Instr->getOperand(0).getReg();
@@ -709,7 +711,9 @@ void SIFixSGPRCopies::processPHINode(MachineInstr &MI) {
           !TRI->isSGPRReg(*MRI, UseMI->getOperand(0).getReg())) {
           numVGPRUses++;
         }
-        worklist.insert(UseMI);
+        if (Visited.insert(UseMI).second)
+          worklist.insert(UseMI);
+
         continue;
       }
 

diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index f75da51..3bcee08 100644 (file)
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -4289,6 +4289,8 @@ void SIInstrInfo::legalizeGenericOperand(MachineBasicBlock &InsertMBB,
 
   bool ImpDef = Def->isImplicitDef();
   while (!ImpDef && Def && Def->isCopy()) {
+    if (Def->getOperand(1).getReg().isPhysical())
+      break;
     Def = MRI.getUniqueVRegDef(Def->getOperand(1).getReg());
     ImpDef = Def && Def->isImplicitDef();
   }

diff --git a/llvm/test/CodeGen/AMDGPU/fix-sgpr-copies.mir b/llvm/test/CodeGen/AMDGPU/fix-sgpr-copies.mir
index 306e62a..394df72 100644 (file)
--- a/llvm/test/CodeGen/AMDGPU/fix-sgpr-copies.mir
+++ b/llvm/test/CodeGen/AMDGPU/fix-sgpr-copies.mir
@@ -60,3 +60,53 @@ body:               |
 
   bb.8:
 ...
+
+# Avoid infinite loop in SIInstrInfo::legalizeGenericOperand when checking for ImpDef.
+# GCN-LABEL: name: legalize-operand-search-each-def-once
+# GCN-NOT: sreg_64 PHI
+---
+name: legalize-operand-search-each-def-once
+tracksRegLiveness: true
+body:               |
+  bb.0:
+    successors: %bb.1, %bb.2
+    liveins: $sgpr0_sgpr1
+
+    %0:sgpr_64 = COPY $sgpr0_sgpr1
+    S_CBRANCH_VCCZ %bb.2, implicit undef $vcc
+    S_BRANCH %bb.1
+
+  bb.1:
+    %1:vreg_64 = IMPLICIT_DEF
+    S_BRANCH %bb.2
+
+  bb.2:
+    %2:sgpr_64 = PHI %0, %bb.0, %1, %bb.1
+    $sgpr0_sgpr1 = COPY %0
+...
+
+# A REG_SEQUENCE that uses registers defined by both a PHI and a COPY could
+# result in an endless search.
+# GCN-LABEL: name: process-phi-search-each-use-once
+# GCN-NOT: sreg_32 PHI
+---
+name: process-phi-search-each-use-once
+tracksRegLiveness: true
+body:               |
+  bb.0:
+    successors: %bb.1, %bb.2
+    liveins: $vgpr3
+
+    %0:vgpr_32 = COPY $vgpr3
+    S_CBRANCH_VCCZ %bb.2, implicit undef $vcc
+    S_BRANCH %bb.1
+
+  bb.1:
+    %1:sgpr_32 = IMPLICIT_DEF
+    S_BRANCH %bb.2
+
+  bb.2:
+    %2:sgpr_32 = PHI %0, %bb.0, %1, %bb.1
+    %3:vreg_64 = REG_SEQUENCE %2, %subreg.sub0, %0, %subreg.sub1
+    $vgpr3 = COPY %3.sub0
+...