[llvm] [SPIR-V] Ensure that internal intrinsic functions are inserted at the correct positions (PR #93552)
via llvm-commits
llvm-commits at lists.llvm.org
Tue May 28 07:09:39 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-spir-v
Author: Vyacheslav Levytskyy (VyacheslavLevytskyy)
<details>
<summary>Changes</summary>
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
---
Full diff: https://github.com/llvm/llvm-project/pull/93552.diff
2 Files Affected:
- (modified) llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp (+32-11)
- (added) llvm/test/CodeGen/SPIRV/phi-spvintrinsic-dominate.ll (+39)
``````````diff
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
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/93552
More information about the llvm-commits
mailing list