[llvm] [SPIR-V] Ensure that internal intrinsic functions are inserted at the correct positions (PR #93552)
Vyacheslav Levytskyy via llvm-commits
llvm-commits at lists.llvm.org
Tue May 28 07:09:02 PDT 2024
https://github.com/VyacheslavLevytskyy created https://github.com/llvm/llvm-project/pull/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
>From e7785ac5c4b8c89432b882e5b7887d4afeff009f Mon Sep 17 00:00:00 2001
From: "Levytskyy, Vyacheslav" <vyacheslav.levytskyy at intel.com>
Date: Tue, 28 May 2024 07:07:34 -0700
Subject: [PATCH] Ensure that internal intrinsic functions are inserted at the
correct positions
---
llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp | 43 ++++++++++++++-----
.../SPIRV/phi-spvintrinsic-dominate.ll | 39 +++++++++++++++++
2 files changed, 71 insertions(+), 11 deletions(-)
create mode 100644 llvm/test/CodeGen/SPIRV/phi-spvintrinsic-dominate.ll
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