[SPIR-V] Ensure that internal intrinsic functions "ptrcast" for PHI's operand are inserted at the correct positions

[VyacheslavLevytskyy]

The goal of the PR is to ensure that newly inserted ptrcast internal intrinsic functions are inserted at the correct positions, and don't break rules of instruction domination and PHI nodes grouping at top of basic block.

[llvmbot]

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

Author: Vyacheslav Levytskyy (VyacheslavLevytskyy)

Changes

The goal of the PR is to ensure that newly inserted ptrcast internal intrinsic functions are inserted at the correct positions, and don't break rules of instruction domination and PHI nodes grouping at top of basic block.

---

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

2 Files Affected:

• (modified) llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp (+13-4)
• (added) llvm/test/CodeGen/SPIRV/phi-ptrcast-dominate.ll (+94)

diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index 32df2403dfe52..a1a08c5c699b6 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -489,10 +489,6 @@ void SPIRVEmitIntrinsics::deduceOperandElementType(Instruction *I) {
     Type *Ty = GR->findDeducedElementType(Op);
     if (Ty == KnownElemTy)
       continue;
-    if (Instruction *User = dyn_cast<Instruction>(Op->use_begin()->get()))
-      setInsertPointSkippingPhis(B, User->getNextNode());
-    else
-      setInsertPointSkippingPhis(B, I);
     Value *OpTyVal = Constant::getNullValue(KnownElemTy);
     Type *OpTy = Op->getType();
     if (!Ty) {
@@ -500,6 +496,8 @@ void SPIRVEmitIntrinsics::deduceOperandElementType(Instruction *I) {
       // check if there is existing Intrinsic::spv_assign_ptr_type instruction
       auto It = AssignPtrTypeInstr.find(Op);
       if (It == AssignPtrTypeInstr.end()) {
+        Instruction *User = dyn_cast<Instruction>(Op->use_begin()->get());
+        setInsertPointSkippingPhis(B, User ? User->getNextNode() : I);
         CallInst *CI =
             buildIntrWithMD(Intrinsic::spv_assign_ptr_type, {OpTy}, OpTyVal, Op,
                             {B.getInt32(getPointerAddressSpace(OpTy))}, B);
@@ -511,6 +509,17 @@ void SPIRVEmitIntrinsics::deduceOperandElementType(Instruction *I) {
                 Ctx, MDNode::get(Ctx, ValueAsMetadata::getConstant(OpTyVal))));
       }
     } else {
+      if (auto *OpI = dyn_cast<Instruction>(Op)) {
+        // spv_ptrcast's argument Op denotes an instruction that generates
+        // a value, and we may use getInsertionPointAfterDef()
+        B.SetInsertPoint(*OpI->getInsertionPointAfterDef());
+        B.SetCurrentDebugLocation(OpI->getDebugLoc());
+      } else if (auto *OpA = dyn_cast<Argument>(Op)) {
+        B.SetInsertPointPastAllocas(OpA->getParent());
+        B.SetCurrentDebugLocation(DebugLoc());
+      } else {
+        B.SetInsertPoint(F->getEntryBlock().getFirstNonPHIOrDbgOrAlloca());
+      }
       SmallVector<Type *, 2> Types = {OpTy, OpTy};
       MetadataAsValue *VMD = MetadataAsValue::get(
           Ctx, MDNode::get(Ctx, ValueAsMetadata::getConstant(OpTyVal)));
diff --git a/llvm/test/CodeGen/SPIRV/phi-ptrcast-dominate.ll b/llvm/test/CodeGen/SPIRV/phi-ptrcast-dominate.ll
new file mode 100644
index 0000000000000..2cd321b05a403
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/phi-ptrcast-dominate.ll
@@ -0,0 +1,94 @@
+; The goal of the test is to check that newly inserted `ptrcast` internal
+; intrinsic functions for PHI's operands are inserted at the correct
+; positions, and don't break rules of instruction domination and PHI nodes
+; grouping at top of basic block.
+
+; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+
+; CHECK-DAG: OpName %[[#Case1:]] "case1"
+; CHECK-DAG: OpName %[[#Case2:]] "case2"
+; CHECK-DAG: OpName %[[#Case3:]] "case3"
+; CHECK: %[[#Case1]] = OpFunction
+; CHECK: OpBranchConditional
+; CHECK: OpPhi
+; CHECK: OpBranch
+; CHECK-COUNT-2: OpBranchConditional
+; CHECK: OpFunctionEnd
+; CHECK: %[[#Case2]] = OpFunction
+; CHECK: OpBranchConditional
+; CHECK: OpPhi
+; CHECK: OpBranch
+; CHECK-COUNT-2: OpBranchConditional
+; CHECK: OpFunctionEnd
+; CHECK: %[[#Case3]] = OpFunction
+; CHECK: OpBranchConditional
+; CHECK: OpPhi
+; CHECK: OpBranch
+; CHECK: OpInBoundsPtrAccessChain
+; CHECK: OpBranchConditional
+; CHECK: OpInBoundsPtrAccessChain
+; CHECK: OpBranchConditional
+; CHECK: OpFunctionEnd
+
+%struct1 = type { i64 }
+%struct2 = type { i64, i64 }
+
+@.str.1 = private unnamed_addr addrspace(1) constant [3 x i8] c"OK\00", align 1
+@.str.2 = private unnamed_addr addrspace(1) constant [6 x i8] c"WRONG\00", align 1
+
+define spir_func void @case1(i1 %b1, i1 %b2, i1 %b3) {
+entry:
+  br i1 %b1, label %l1, label %l2
+
+l1:
+  %str = phi ptr addrspace(1) [ @.str.1, %entry ], [ @.str.2, %l2 ], [ @.str.2, %l3 ]
+  br label %exit
+
+l2:
+  br i1 %b2, label %l1, label %l3
+
+l3:
+  br i1 %b3, label %l1, label %exit
+
+exit:
+  ret void
+}
+
+define spir_func void @case2(i1 %b1, i1 %b2, i1 %b3, ptr addrspace(1) byval(%struct1) %str1, ptr addrspace(1) byval(%struct2) %str2) {
+entry:
+  br i1 %b1, label %l1, label %l2
+
+l1:
+  %str = phi ptr addrspace(1) [ %str1, %entry ], [ %str2, %l2 ], [ %str2, %l3 ]
+  br label %exit
+
+l2:
+  br i1 %b2, label %l1, label %l3
+
+l3:
+  br i1 %b3, label %l1, label %exit
+
+exit:
+  ret void
+}
+
+define spir_func void @case3(i1 %b1, i1 %b2, i1 %b3, ptr addrspace(1) byval(%struct1) %_arg_str1, ptr addrspace(1) byval(%struct2) %_arg_str2) {
+entry:
+  br i1 %b1, label %l1, label %l2
+
+l1:
+  %str = phi ptr addrspace(1) [ %_arg_str1, %entry ], [ %str2, %l2 ], [ %str3, %l3 ]
+  br label %exit
+
+l2:
+  %str2 = getelementptr inbounds %struct2, ptr addrspace(1) %_arg_str2, i32 1
+  br i1 %b2, label %l1, label %l3
+
+l3:
+  %str3 = getelementptr inbounds %struct2, ptr addrspace(1) %_arg_str2, i32 2
+  br i1 %b3, label %l1, label %exit
+
+exit:
+  ret void
+}