[PATCH] D64393: [AMDGPU] Fix DPP combiner check for exec modification

[Jay Foad via Phabricator via llvm-commits]

foad created this revision.
foad added reviewers: arsenm, vpykhtin.
Herald added subscribers: jfb, MaskRay, kbarton, hiraditya, t-tye, tpr, dstuttard, yaxunl, nhaehnle, wdng, jvesely, nemanjai, kzhuravl.
Herald added a project: LLVM.

r363675 changed the exec modification helper function, now called
execMayBeModifiedBeforeUse, so that if no UseMI is specified it checks
all instructions in the basic block, even beyond the last use. That
meant that the DPP combiner no longer worked in any basic block that
ended with a control flow instruction, and in particular it didn't work
on code sequences generated by the atomic optimizer.

Fix it by changing execMayBeModifiedBeforeUse to require UseMI and
changing the DPP combiner to check each use individually.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64393

Files:
  llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp
  llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
  llvm/test/CodeGen/AMDGPU/atomic_optimizations_local_pointer.ll


Index: llvm/test/CodeGen/AMDGPU/atomic_optimizations_local_pointer.ll
===================================================================
--- llvm/test/CodeGen/AMDGPU/atomic_optimizations_local_pointer.ll
+++ llvm/test/CodeGen/AMDGPU/atomic_optimizations_local_pointer.ll
@@ -190,3 +190,17 @@
   store i64 %old, i64 addrspace(1)* %out
   ret void
 }
+
+; Check that the DPP combiner works on the code generated by the atomic optimizer.
+
+; GFX8MORE-LABEL: add_i32_varying_dpp:
+; GFX8MORE: v_add_u32_dpp
+; GFX8MORE: v_add_u32_dpp
+; GFX8MORE: v_add_u32_dpp
+; GFX8MORE: v_add_u32_dpp
+; GFX8MORE: v_add_u32_dpp
+define amdgpu_kernel void @add_i32_varying_dpp() {
+  %lane = call i32 @llvm.amdgcn.workitem.id.x()
+  %old = atomicrmw add i32 addrspace(3)* @local_var32, i32 %lane acq_rel
+  ret void
+}
Index: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
===================================================================
--- llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -6091,29 +6091,16 @@
   auto *TRI = MRI.getTargetRegisterInfo();
   auto *DefBB = DefMI.getParent();
 
-  if (UseMI) {
-    // Don't bother searching between blocks, although it is possible this block
-    // doesn't modify exec.
-    if (UseMI->getParent() != DefBB)
-      return true;
-  } else {
-    int NumUse = 0;
-    const int MaxUseScan = 10;
-
-    for (auto &UseInst : MRI.use_nodbg_instructions(VReg)) {
-      if (UseInst.getParent() != DefBB)
-        return true;
-
-      if (NumUse++ > MaxUseScan)
-        return true;
-    }
-  }
+  // Don't bother searching between blocks, although it is possible this block
+  // doesn't modify exec.
+  if (UseMI->getParent() != DefBB)
+    return true;
 
   const int MaxInstScan = 20;
   int NumScan = 0;
 
   // Stop scan at the use if known.
-  auto E = UseMI ? UseMI->getIterator() : DefBB->end();
+  auto E = UseMI->getIterator();
   for (auto I = std::next(DefMI.getIterator()); I != E; ++I) {
     if (I->isDebugInstr())
       continue;
Index: llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp
===================================================================
--- llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp
+++ llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp
@@ -344,11 +344,6 @@
   auto *DstOpnd = TII->getNamedOperand(MovMI, AMDGPU::OpName::vdst);
   assert(DstOpnd && DstOpnd->isReg());
   auto DPPMovReg = DstOpnd->getReg();
-  if (execMayBeModifiedBeforeUse(*MRI, DPPMovReg, MovMI)) {
-    LLVM_DEBUG(dbgs() << "  failed: EXEC mask should remain the same"
-                         " for all uses\n");
-    return false;
-  }
 
   auto *RowMaskOpnd = TII->getNamedOperand(MovMI, AMDGPU::OpName::row_mask);
   assert(RowMaskOpnd && RowMaskOpnd->isImm());
@@ -426,6 +421,12 @@
     auto &OrigMI = *Use.getParent();
     LLVM_DEBUG(dbgs() << "  try: " << OrigMI);
 
+    if (execMayBeModifiedBeforeUse(*MRI, DPPMovReg, MovMI, &OrigMI)) {
+      LLVM_DEBUG(dbgs() << "  failed: EXEC mask should remain the same"
+                 " for all uses\n");
+      break;
+    }
+
     auto OrigOp = OrigMI.getOpcode();
     if (TII->isVOP3(OrigOp)) {
       if (!TII->hasVALU32BitEncoding(OrigOp)) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D64393.208614.patch
Type: text/x-patch
Size: 3150 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190709/4beb2362/attachment.bin>