Skip to content

Commit c1dc946

Browse files
VyacheslavLevytskyyjsji
authored andcommitted
Ensure that PHI node has an incoming value per each predecessor instance (#2736)
This PR partially fixes issue #2702 in the part that is responsible for SPIR-V to LLVM IR translation. Namely, this PR ensures that all PHI nodes of a Function has the number of incoming blocks matching block's predecessor count. When a PHI node doesn't conform to this rule, this PR inserts missing number of (Value, Basic Block) pairs to make the PHI node valid. Another problem from #2702, that is violation of the requirement to OpPhi's to have exactly one Parent ID operand for each parent block of the current block in the CFG in the output SPIR-V code, is out of scope of this PR. Original commit: KhronosGroup/SPIRV-LLVM-Translator@7d7f946dbd51236
1 parent 85f42e6 commit c1dc946

File tree

2 files changed

+78
-0
lines changed

2 files changed

+78
-0
lines changed

llvm-spirv/lib/SPIRV/SPIRVReader.cpp

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3073,6 +3073,46 @@ void SPIRVToLLVM::transFunctionAttrs(SPIRVFunction *BF, Function *F) {
30733073
});
30743074
}
30753075

3076+
namespace {
3077+
// One basic block can be a predecessor to another basic block more than
3078+
// once (https://github.com/KhronosGroup/SPIRV-LLVM-Translator/issues/2702).
3079+
// This function fixes any PHIs that break this rule.
3080+
static void validatePhiPredecessors(Function *F) {
3081+
for (BasicBlock &BB : *F) {
3082+
bool UniquePreds = true;
3083+
DenseMap<BasicBlock *, unsigned> PredsCnt;
3084+
for (BasicBlock *PredBB : predecessors(&BB)) {
3085+
auto It = PredsCnt.try_emplace(PredBB, 1);
3086+
if (!It.second) {
3087+
UniquePreds = false;
3088+
++It.first->second;
3089+
}
3090+
}
3091+
if (UniquePreds)
3092+
continue;
3093+
// `phi` requires an incoming value per each predecessor instance, even
3094+
// it's the same basic block that has been already inserted as an incoming
3095+
// value of the `phi`.
3096+
for (PHINode &Phi : BB.phis()) {
3097+
SmallVector<Value *> Vs;
3098+
SmallVector<BasicBlock *> Bs;
3099+
for (auto [V, B] : zip(Phi.incoming_values(), Phi.blocks())) {
3100+
unsigned N = PredsCnt[B];
3101+
Vs.insert(Vs.end(), N, V);
3102+
Bs.insert(Bs.end(), N, B);
3103+
}
3104+
unsigned I = 0;
3105+
for (unsigned N = Phi.getNumIncomingValues(); I < N; ++I) {
3106+
Phi.setIncomingValue(I, Vs[I]);
3107+
Phi.setIncomingBlock(I, Bs[I]);
3108+
}
3109+
for (unsigned N = Vs.size(); I < N; ++I)
3110+
Phi.addIncoming(Vs[I], Bs[I]);
3111+
}
3112+
}
3113+
}
3114+
} // namespace
3115+
30763116
Function *SPIRVToLLVM::transFunction(SPIRVFunction *BF, unsigned AS) {
30773117
auto Loc = FuncMap.find(BF);
30783118
if (Loc != FuncMap.end())
@@ -3186,6 +3226,7 @@ Function *SPIRVToLLVM::transFunction(SPIRVFunction *BF, unsigned AS) {
31863226
}
31873227
}
31883228

3229+
validatePhiPredecessors(F);
31893230
transLLVMLoopMetadata(F);
31903231

31913232
return F;
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
; REQUIRES: spirv-as
2+
; RUN: spirv-as --target-env spv1.0 -o %t.spv %s
3+
; RUN: spirv-val %t.spv
4+
; RUN: llvm-spirv -r -o - %t.spv | llvm-dis | FileCheck %s
5+
6+
OpCapability Kernel
7+
OpCapability Addresses
8+
OpCapability Int64
9+
OpCapability Linkage
10+
%1 = OpExtInstImport "OpenCL.std"
11+
OpMemoryModel Physical64 OpenCL
12+
OpSource OpenCL_CPP 100000
13+
OpName %foo "foo"
14+
OpName %get "get"
15+
OpDecorate %foo LinkageAttributes "foo" Export
16+
OpDecorate %get LinkageAttributes "get" Import
17+
%uint = OpTypeInt 32 0
18+
%3 = OpTypeFunction %uint
19+
%ulong = OpTypeInt 64 0
20+
%uint_2 = OpConstant %uint 2
21+
%uint_4 = OpConstant %uint 4
22+
%get = OpFunction %uint None %3
23+
OpFunctionEnd
24+
%foo = OpFunction %uint None %3
25+
%11 = OpLabel
26+
%9 = OpFunctionCall %uint %get
27+
OpSwitch %9 %12 10 %13 4 %13 0 %13 42 %13
28+
%12 = OpLabel
29+
OpBranch %13
30+
%13 = OpLabel
31+
%10 = OpPhi %uint %uint_4 %11 %uint_2 %12
32+
%14 = OpPhi %uint %uint_2 %12 %uint_4 %11
33+
OpReturnValue %14
34+
OpFunctionEnd
35+
36+
; CHECK: phi i32 [ 4, %0 ], [ 4, %0 ], [ 4, %0 ], [ 4, %0 ], [ 2, %2 ]
37+
; CHECK: phi i32 [ 2, %2 ], [ 4, %0 ], [ 4, %0 ], [ 4, %0 ], [ 4, %0 ]

0 commit comments

Comments
 (0)