[llvm] 7fa45af - [SPIR-V] Ensure that internal intrinsic functions are inserted at the correct positions (#93552)

via llvm-commits llvm-commits at lists.llvm.org
Wed May 29 03:52:59 PDT 2024


Author: Vyacheslav Levytskyy
Date: 2024-05-29T12:52:55+02:00
New Revision: 7fa45afa938e0feb0030b14a8633de7dd8e529cb

URL: https://github.com/llvm/llvm-project/commit/7fa45afa938e0feb0030b14a8633de7dd8e529cb
DIFF: https://github.com/llvm/llvm-project/commit/7fa45afa938e0feb0030b14a8633de7dd8e529cb.diff

LOG: [SPIR-V] Ensure that internal intrinsic functions are inserted at the correct positions (#93552)

The goal of the PR is to ensure that newly inserted 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.
This is a continuation of
https://github.com/llvm/llvm-project/pull/92316 and
https://github.com/llvm/llvm-project/pull/92536

Added: 
    llvm/test/CodeGen/SPIRV/phi-spvintrinsic-dominate.ll

Modified: 
    llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index ea53fe55e7ab5..e4bbeb53d1691 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -181,6 +181,14 @@ static void setInsertPointSkippingPhis(IRBuilder<> &B, Instruction *I) {
     B.SetInsertPoint(I);
 }
 
+static void setInsertPointAfterDef(IRBuilder<> &B, Instruction *I) {
+  B.SetCurrentDebugLocation(I->getDebugLoc());
+  if (I->getType()->isVoidTy())
+    B.SetInsertPoint(I->getNextNode());
+  else
+    B.SetInsertPoint(*I->getInsertionPointAfterDef());
+}
+
 static bool requireAssignType(Instruction *I) {
   IntrinsicInst *Intr = dyn_cast<IntrinsicInst>(I);
   if (Intr) {
@@ -560,6 +568,7 @@ void SPIRVEmitIntrinsics::preprocessUndefs(IRBuilder<> &B) {
 
   while (!Worklist.empty()) {
     Instruction *I = Worklist.front();
+    bool BPrepared = false;
     Worklist.pop();
 
     for (auto &Op : I->operands()) {
@@ -567,7 +576,10 @@ void SPIRVEmitIntrinsics::preprocessUndefs(IRBuilder<> &B) {
       if (!AggrUndef || !Op->getType()->isAggregateType())
         continue;
 
-      B.SetInsertPoint(I);
+      if (!BPrepared) {
+        setInsertPointSkippingPhis(B, I);
+        BPrepared = true;
+      }
       auto *IntrUndef = B.CreateIntrinsic(Intrinsic::spv_undef, {}, {});
       Worklist.push(IntrUndef);
       I->replaceUsesOfWith(Op, IntrUndef);
@@ -584,6 +596,7 @@ void SPIRVEmitIntrinsics::preprocessCompositeConstants(IRBuilder<> &B) {
 
   while (!Worklist.empty()) {
     auto *I = Worklist.front();
+    bool IsPhi = isa<PHINode>(I), BPrepared = false;
     assert(I);
     bool KeepInst = false;
     for (const auto &Op : I->operands()) {
@@ -615,7 +628,11 @@ void SPIRVEmitIntrinsics::preprocessCompositeConstants(IRBuilder<> &B) {
         else
           for (auto &COp : AggrConst->operands())
             Args.push_back(COp);
-        B.SetInsertPoint(I);
+        if (!BPrepared) {
+          IsPhi ? B.SetInsertPointPastAllocas(I->getParent()->getParent())
+                : B.SetInsertPoint(I);
+          BPrepared = true;
+        }
         auto *CI =
             B.CreateIntrinsic(Intrinsic::spv_const_composite, {ResTy}, {Args});
         Worklist.push(CI);
@@ -1111,8 +1128,7 @@ void SPIRVEmitIntrinsics::insertAssignPtrTypeIntrs(Instruction *I,
       isa<BitCastInst>(I))
     return;
 
-  setInsertPointSkippingPhis(B, I->getNextNode());
-
+  setInsertPointAfterDef(B, I);
   Type *ElemTy = deduceElementType(I);
   Constant *EltTyConst = UndefValue::get(ElemTy);
   unsigned AddressSpace = getPointerAddressSpace(I->getType());
@@ -1127,7 +1143,7 @@ void SPIRVEmitIntrinsics::insertAssignTypeIntrs(Instruction *I,
   reportFatalOnTokenType(I);
   Type *Ty = I->getType();
   if (!Ty->isVoidTy() && !isPointerTy(Ty) && requireAssignType(I)) {
-    setInsertPointSkippingPhis(B, I->getNextNode());
+    setInsertPointAfterDef(B, I);
     Type *TypeToAssign = Ty;
     if (auto *II = dyn_cast<IntrinsicInst>(I)) {
       if (II->getIntrinsicID() == Intrinsic::spv_const_composite ||
@@ -1149,7 +1165,7 @@ void SPIRVEmitIntrinsics::insertAssignTypeIntrs(Instruction *I,
       if (isa<UndefValue>(Op) && Op->getType()->isAggregateType())
         buildIntrWithMD(Intrinsic::spv_assign_type, {B.getInt32Ty()}, Op,
                         UndefValue::get(B.getInt32Ty()), {}, B);
-      else if (!isa<Instruction>(Op)) // TODO: This case could be removed
+      else if (!isa<Instruction>(Op))
         buildIntrWithMD(Intrinsic::spv_assign_type, {Op->getType()}, Op, Op, {},
                         B);
     }
@@ -1159,7 +1175,7 @@ void SPIRVEmitIntrinsics::insertAssignTypeIntrs(Instruction *I,
 void SPIRVEmitIntrinsics::insertSpirvDecorations(Instruction *I,
                                                  IRBuilder<> &B) {
   if (MDNode *MD = I->getMetadata("spirv.Decorations")) {
-    B.SetInsertPoint(I->getNextNode());
+    setInsertPointAfterDef(B, I);
     B.CreateIntrinsic(Intrinsic::spv_assign_decoration, {I->getType()},
                       {I, MetadataAsValue::get(I->getContext(), MD)});
   }
@@ -1170,7 +1186,7 @@ void SPIRVEmitIntrinsics::processInstrAfterVisit(Instruction *I,
   auto *II = dyn_cast<IntrinsicInst>(I);
   if (II && II->getIntrinsicID() == Intrinsic::spv_const_composite &&
       TrackConstants) {
-    B.SetInsertPoint(I->getNextNode());
+    setInsertPointAfterDef(B, I);
     auto t = AggrConsts.find(I);
     assert(t != AggrConsts.end());
     auto *NewOp =
@@ -1179,6 +1195,7 @@ void SPIRVEmitIntrinsics::processInstrAfterVisit(Instruction *I,
     I->replaceAllUsesWith(NewOp);
     NewOp->setArgOperand(0, I);
   }
+  bool IsPhi = isa<PHINode>(I), BPrepared = false;
   for (const auto &Op : I->operands()) {
     if ((isa<ConstantAggregateZero>(Op) && Op->getType()->isVectorTy()) ||
         isa<PHINode>(I) || isa<SwitchInst>(I))
@@ -1188,7 +1205,11 @@ void SPIRVEmitIntrinsics::processInstrAfterVisit(Instruction *I,
       if (II && ((II->getIntrinsicID() == Intrinsic::spv_gep && OpNo == 0) ||
                  (II->paramHasAttr(OpNo, Attribute::ImmArg))))
         continue;
-      B.SetInsertPoint(I);
+      if (!BPrepared) {
+        IsPhi ? B.SetInsertPointPastAllocas(I->getParent()->getParent())
+              : B.SetInsertPoint(I);
+        BPrepared = true;
+      }
       Value *OpTyVal = Op;
       if (Op->getType()->isTargetExtTy())
         OpTyVal = Constant::getNullValue(
@@ -1201,7 +1222,7 @@ void SPIRVEmitIntrinsics::processInstrAfterVisit(Instruction *I,
   }
   if (I->hasName()) {
     reportFatalOnTokenType(I);
-    setInsertPointSkippingPhis(B, I->getNextNode());
+    setInsertPointAfterDef(B, I);
     std::vector<Value *> Args = {I};
     addStringImm(I->getName(), B, Args);
     B.CreateIntrinsic(Intrinsic::spv_assign_name, {I->getType()}, Args);
@@ -1345,7 +1366,7 @@ bool SPIRVEmitIntrinsics::runOnFunction(Function &Func) {
   for (auto *I : Worklist) {
     TrackConstants = true;
     if (!I->getType()->isVoidTy() || isa<StoreInst>(I))
-      B.SetInsertPoint(I->getNextNode());
+      setInsertPointAfterDef(B, I);
     // Visitors return either the original/newly created instruction for further
     // processing, nullptr otherwise.
     I = visit(*I);

diff  --git a/llvm/test/CodeGen/SPIRV/phi-spvintrinsic-dominate.ll b/llvm/test/CodeGen/SPIRV/phi-spvintrinsic-dominate.ll
new file mode 100644
index 0000000000000..471ab03ed89f6
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/phi-spvintrinsic-dominate.ll
@@ -0,0 +1,39 @@
+; The goal of the test is to check that newly inserted internal (spv)
+; 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: OpFunction
+; CHECK: OpBranch
+; CHECK: OpLabel
+; CHECK: OpPhi
+; CHECK: OpPhi
+; CHECK: OpPhi
+
+define spir_kernel void @foo(ptr addrspace(1) %_arg1) {
+entry:
+  br label %l1
+
+l1:
+  %sw = phi <4 x double> [ %vec, %l2 ], [ <double 0.0, double 0.0, double 0.0, double poison>, %entry ]
+  %in = phi <3 x double> [ %ins, %l2 ], [ zeroinitializer, %entry ]
+  %r1 = phi i32 [ %r2, %l2 ], [ 0, %entry ]
+  %c1 = icmp ult i32 %r1, 3
+  br i1 %c1, label %l2, label %exit
+
+l2:
+  %r3 = zext nneg i32 %r1 to i64
+  %r4 = getelementptr inbounds double, ptr addrspace(1) %_arg1, i64 %r3
+  %r5 = load double, ptr addrspace(1) %r4, align 8
+  %ins = insertelement <3 x double> %in, double %r5, i32 %r1
+  %exp = shufflevector <3 x double> %ins, <3 x double> poison, <4 x i32> <i32 0, i32 1, i32 2, i32 poison>
+  %vec = shufflevector <4 x double> %exp, <4 x double> %sw, <4 x i32> <i32 0, i32 1, i32 2, i32 7>
+  %r2 = add nuw nsw i32 %r1, 1
+  br label %l1
+
+exit:
+  ret void
+}


        


More information about the llvm-commits mailing list