[AMDGPU] Prevent re-visits in LowerBufferFatPointers

[krzysz00]

Fixes iree-org/iree#22001

The visitor in SplitPtrStructs would re-visit instructions if an instruction earlier in program order caused a recursive visit() call via getPtrParts(). This would cause instructions to be processed multiple times.

As a consequence of this, PHI nodes could be added to the Conditionals array multiple times, which would to a conditinoal that was already simplified being processed multiple times. After the code moved to InstSimplifyFolder, this re-processing, combined with more agressive simplifications, would lead to an attempt to replace an instruction with itself, causing an assertion failure and crash.

This commit resolves the issue and adds the reduced form of the crashing input as a test.

[llvmbot]

@llvm/pr-subscribers-backend-amdgpu

Author: Krzysztof Drewniak (krzysz00)

Changes

Fixes iree-org/iree#22001

The visitor in SplitPtrStructs would re-visit instructions if an instruction earlier in program order caused a recursive visit() call via getPtrParts(). This would cause instructions to be processed multiple times.

As a consequence of this, PHI nodes could be added to the Conditionals array multiple times, which would to a conditinoal that was already simplified being processed multiple times. After the code moved to InstSimplifyFolder, this re-processing, combined with more agressive simplifications, would lead to an attempt to replace an instruction with itself, causing an assertion failure and crash.

This commit resolves the issue and adds the reduced form of the crashing input as a test.

---

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

2 Files Affected:

• (modified) llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp (+6)
• (modified) llvm/test/CodeGen/AMDGPU/lower-buffer-fat-pointers-control-flow.ll (+26)

diff --git a/llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp b/llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp
index 7dc1ec07cf0f9..d9bfeae52e213 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp
@@ -2328,6 +2328,12 @@ void SplitPtrStructs::processFunction(Function &F) {
   LLVM_DEBUG(dbgs() << "Splitting pointer structs in function: " << F.getName()
                     << "\n");
   for (Instruction *I : Originals) {
+    // In some cases, instruction order doesn't reflect program order,
+    // so the visit() call will have already visited coertain instructions
+    // by the time this loop gets to them. Avoid re-visiting these so as to,
+    // for example, avoid processing the same conditional twice.
+    if (SplitUsers.contains(I))
+      continue;
     auto [Rsrc, Off] = visit(I);
     assert(((Rsrc && Off) || (!Rsrc && !Off)) &&
            "Can't have a resource but no offset");
diff --git a/llvm/test/CodeGen/AMDGPU/lower-buffer-fat-pointers-control-flow.ll b/llvm/test/CodeGen/AMDGPU/lower-buffer-fat-pointers-control-flow.ll
index 29e727b393309..4fa7c29bfde02 100644
--- a/llvm/test/CodeGen/AMDGPU/lower-buffer-fat-pointers-control-flow.ll
+++ b/llvm/test/CodeGen/AMDGPU/lower-buffer-fat-pointers-control-flow.ll
@@ -455,3 +455,29 @@ loop:
 exit:
   ret float %sum
 }
+
+define void @dominance_not_in_program_order(ptr addrspace(7) inreg %arg) {
+; CHECK-LABEL: define void @dominance_not_in_program_order
+; CHECK-SAME: ({ ptr addrspace(8), i32 } inreg [[ARG:%.*]]) #[[ATTR0]] {
+; CHECK-NEXT:  .preheader15:
+; CHECK-NEXT:    [[ARG_RSRC:%.*]] = extractvalue { ptr addrspace(8), i32 } [[ARG]], 0
+; CHECK-NEXT:    [[ARG_OFF:%.*]] = extractvalue { ptr addrspace(8), i32 } [[ARG]], 1
+; CHECK-NEXT:    br label [[DOTLR_PH18:%.*]]
+; CHECK:       .loopexit:
+; CHECK-NEXT:    [[SCEVGEP12:%.*]] = add i32 [[LSR_IV11_OFF:%.*]], 16
+; CHECK-NEXT:    br label [[DOTLR_PH18]]
+; CHECK:       .lr.ph18:
+; CHECK-NEXT:    [[LSR_IV11_OFF]] = phi i32 [ [[ARG_OFF]], [[DOTLOOPEXIT:%.*]] ], [ [[ARG_OFF]], [[DOTPREHEADER15:%.*]] ]
+; CHECK-NEXT:    br label [[DOTLOOPEXIT]]
+;
+.preheader15:
+  br label %.lr.ph18
+
+.loopexit:                                        ; preds = %.lr.ph18
+  %scevgep12 = getelementptr i8, ptr addrspace(7) %lsr.iv11, i32 16
+  br label %.lr.ph18
+
+.lr.ph18:                                         ; preds = %.loopexit, %.preheader15
+  %lsr.iv11 = phi ptr addrspace(7) [ %arg, %.loopexit ], [ %arg, %.preheader15 ]
+  br label %.loopexit
+}