Ensure that PHI node has an incoming value per each predecessor insta…

…nce (#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.

lib/SPIRV/SPIRVReader.cpp

@@ -3125,6 +3125,46 @@ void SPIRVToLLVM::transFunctionAttrs(SPIRVFunction *BF, Function *F) {
   });
 }
 
+namespace {
+// One basic block can be a predecessor to another basic block more than
+// once (https://github.com/KhronosGroup/SPIRV-LLVM-Translator/issues/2702).
+// This function fixes any PHIs that break this rule.
+static void validatePhiPredecessors(Function *F) {
+  for (BasicBlock &BB : *F) {
+    bool UniquePreds = true;
+    DenseMap<BasicBlock *, unsigned> PredsCnt;
+    for (BasicBlock *PredBB : predecessors(&BB)) {
+      auto It = PredsCnt.try_emplace(PredBB, 1);
+      if (!It.second) {
+        UniquePreds = false;
+        ++It.first->second;
+      }
+    }
+    if (UniquePreds)
+      continue;
+    // `phi` requires an incoming value per each predecessor instance, even
+    // it's the same basic block that has been already inserted as an incoming
+    // value of the `phi`.
+    for (PHINode &Phi : BB.phis()) {
+      SmallVector<Value *> Vs;
+      SmallVector<BasicBlock *> Bs;
+      for (auto [V, B] : zip(Phi.incoming_values(), Phi.blocks())) {
+        unsigned N = PredsCnt[B];
+        Vs.insert(Vs.end(), N, V);
+        Bs.insert(Bs.end(), N, B);
+      }
+      unsigned I = 0;
+      for (unsigned N = Phi.getNumIncomingValues(); I < N; ++I) {
+        Phi.setIncomingValue(I, Vs[I]);
+        Phi.setIncomingBlock(I, Bs[I]);
+      }
+      for (unsigned N = Vs.size(); I < N; ++I)
+        Phi.addIncoming(Vs[I], Bs[I]);
+    }
+  }
+}
+} // namespace
+
 Function *SPIRVToLLVM::transFunction(SPIRVFunction *BF, unsigned AS) {
   auto Loc = FuncMap.find(BF);
   if (Loc != FuncMap.end())
@@ -3209,6 +3249,7 @@ Function *SPIRVToLLVM::transFunction(SPIRVFunction *BF, unsigned AS) {
     }
   }
 
+  validatePhiPredecessors(F);
   transLLVMLoopMetadata(F);
 
   return F;

test/phi-multiple-predecessors.spvasm

@@ -0,0 +1,37 @@
+; REQUIRES: spirv-as
+; RUN: spirv-as --target-env spv1.0 -o %t.spv %s
+; RUN: spirv-val %t.spv
+; RUN: llvm-spirv -r -o - %t.spv | llvm-dis | FileCheck %s
+
+               OpCapability Kernel
+               OpCapability Addresses
+               OpCapability Int64
+               OpCapability Linkage
+          %1 = OpExtInstImport "OpenCL.std"
+               OpMemoryModel Physical64 OpenCL
+               OpSource OpenCL_CPP 100000
+               OpName %foo "foo"
+               OpName %get "get"
+               OpDecorate %foo LinkageAttributes "foo" Export
+               OpDecorate %get LinkageAttributes "get" Import
+       %uint = OpTypeInt 32 0
+          %3 = OpTypeFunction %uint
+      %ulong = OpTypeInt 64 0
+     %uint_2 = OpConstant %uint 2
+     %uint_4 = OpConstant %uint 4
+        %get = OpFunction %uint None %3
+               OpFunctionEnd
+        %foo = OpFunction %uint None %3
+         %11 = OpLabel
+          %9 = OpFunctionCall %uint %get
+               OpSwitch %9 %12 10 %13 4 %13 0 %13 42 %13
+         %12 = OpLabel
+               OpBranch %13
+         %13 = OpLabel
+         %10 = OpPhi %uint %uint_4 %11 %uint_2 %12
+         %14 = OpPhi %uint %uint_2 %12 %uint_4 %11
+               OpReturnValue %14
+               OpFunctionEnd
+
+; CHECK: phi i32 [ 4, %0 ], [ 4, %0 ], [ 4, %0 ], [ 4, %0 ], [ 2, %2 ]
+; CHECK: phi i32 [ 2, %2 ], [ 4, %0 ], [ 4, %0 ], [ 4, %0 ], [ 4, %0 ]