diff --git a/lib/SPIRV/SPIRVWriter.cpp b/lib/SPIRV/SPIRVWriter.cpp index a8f25b8c27..933a7733d0 100644 --- a/lib/SPIRV/SPIRVWriter.cpp +++ b/lib/SPIRV/SPIRVWriter.cpp @@ -2453,7 +2453,11 @@ LLVMToSPIRVBase::transValueWithoutDecoration(Value *V, SPIRVBasicBlock *BB, Function *Fun = Branch->getFunction(); DominatorTree DomTree(*Fun); LoopInfo LI(DomTree); - for (const auto *LoopObj : LI.getLoopsInPreorder()) { + // Find the innermost loop that contains the current basic block + BasicBlock *CurrentBB = Branch->getParent(); + const Loop *ContainingLoop = LI.getLoopFor(CurrentBB); + + if (ContainingLoop) { // Check whether SuccessorFalse or SuccessorTrue is the loop header BB. // For example consider following LLVM IR: // br i1 %compare, label %for.body, label %for.end @@ -2462,12 +2466,12 @@ LLVMToSPIRVBase::transValueWithoutDecoration(Value *V, SPIRVBasicBlock *BB, // <- SuccessorTrue is 'for.end' aka successor(1) // meanwhile the true successor (by definition) should be a loop header // aka 'for.body' - if (LoopObj->getHeader() == Branch->getSuccessor(1)) + if (ContainingLoop->getHeader() == Branch->getSuccessor(1)) // SuccessorFalse is the loop header BB. BM->addLoopMergeInst(SuccessorTrue->getId(), // Merge Block BB->getId(), // Continue Target LoopControl, Parameters, SuccessorFalse); - else + else if (ContainingLoop->getHeader() == Branch->getSuccessor(0)) // SuccessorTrue is the loop header BB. BM->addLoopMergeInst(SuccessorFalse->getId(), // Merge Block BB->getId(), // Continue Target diff --git a/lib/SPIRV/libSPIRV/SPIRVModule.cpp b/lib/SPIRV/libSPIRV/SPIRVModule.cpp index ea82be35e9..7c21e0dfe3 100644 --- a/lib/SPIRV/libSPIRV/SPIRVModule.cpp +++ b/lib/SPIRV/libSPIRV/SPIRVModule.cpp @@ -1855,10 +1855,22 @@ SPIRVInstruction *SPIRVModuleImpl::addSelectionMergeInst( SPIRVInstruction *SPIRVModuleImpl::addLoopMergeInst( SPIRVId MergeBlock, SPIRVId ContinueTarget, SPIRVWord LoopControl, std::vector LoopControlParameters, SPIRVBasicBlock *BB) { - return addInstruction( - new SPIRVLoopMerge(MergeBlock, ContinueTarget, LoopControl, - LoopControlParameters, BB), - BB, const_cast(BB->getTerminateInstr())); + SPIRVInstruction *TermInst = const_cast(BB->getTerminateInstr()); + // OpLoopMerge must be the second-to-last instruction in the block, + // immediately preceding the branch instruction (OpBranch or OpBranchConditional) + if (TermInst && (TermInst->getOpCode() == OpBranch || + TermInst->getOpCode() == OpBranchConditional)) { + return addInstruction( + new SPIRVLoopMerge(MergeBlock, ContinueTarget, LoopControl, + LoopControlParameters, BB), + BB, TermInst); + } else { + // If there's no proper terminator, add at the end + return addInstruction( + new SPIRVLoopMerge(MergeBlock, ContinueTarget, LoopControl, + LoopControlParameters, BB), + BB); + } } SPIRVInstruction *SPIRVModuleImpl::addLoopControlINTELInst( @@ -1866,9 +1878,19 @@ SPIRVInstruction *SPIRVModuleImpl::addLoopControlINTELInst( SPIRVBasicBlock *BB) { addCapability(CapabilityUnstructuredLoopControlsINTEL); addExtension(ExtensionID::SPV_INTEL_unstructured_loop_controls); - return addInstruction( - new SPIRVLoopControlINTEL(LoopControl, LoopControlParameters, BB), BB, - const_cast(BB->getTerminateInstr())); + SPIRVInstruction *TermInst = const_cast(BB->getTerminateInstr()); + // OpLoopControlINTEL must be the second-to-last instruction in the block, + // immediately preceding the branch instruction (OpBranch or OpBranchConditional) + if (TermInst && (TermInst->getOpCode() == OpBranch || + TermInst->getOpCode() == OpBranchConditional)) { + return addInstruction( + new SPIRVLoopControlINTEL(LoopControl, LoopControlParameters, BB), BB, + TermInst); + } else { + // If there's no proper terminator, add at the end + return addInstruction( + new SPIRVLoopControlINTEL(LoopControl, LoopControlParameters, BB), BB); + } } SPIRVInstruction *SPIRVModuleImpl::addFixedPointIntelInst( diff --git a/test/OpLoopMerge_loopMerge.ll b/test/OpLoopMerge_loopMerge.ll new file mode 100644 index 0000000000..efd9eccdce --- /dev/null +++ b/test/OpLoopMerge_loopMerge.ll @@ -0,0 +1,10 @@ +; Test case for OpLoopMerge instruction placement validation. +; This test verifies that OpLoopMerge instructions are properly placed as the second-to-last +; instruction in their basic block, immediately preceding the branch instruction. + +; RUN: llvm-spirv %S/loopMerge.bc -o %t.spv +; RUN: spirv-val %t.spv + +; RUN: llvm-spirv --to-text %t.spv -o - | FileCheck %s --check-prefix=CHECK-SPIRV-TEXT +; CHECK-SPIRV-TEXT: LoopMerge +; CHECK-SPIRV-TEXT: BranchConditional \ No newline at end of file diff --git a/test/OpLoopMerge_mergeBlock.ll b/test/OpLoopMerge_mergeBlock.ll new file mode 100644 index 0000000000..25299b82fb --- /dev/null +++ b/test/OpLoopMerge_mergeBlock.ll @@ -0,0 +1,10 @@ +; Test case for OpLoopMerge merge block assignment validation. +; This test verifies that OpLoopMerge instructions properly assign unique merge blocks +; and don't create conflicts where the same block is used as a merge block for multiple loops. + +; RUN: llvm-spirv %S/mergeBlock.bc -o %t.spv +; RUN: spirv-val %t.spv + +; RUN: llvm-spirv --to-text %t.spv -o - | FileCheck %s --check-prefix=CHECK-SPIRV-TEXT +; CHECK-SPIRV-TEXT: LoopMerge +; CHECK-SPIRV-TEXT: BranchConditional \ No newline at end of file diff --git a/test/loopMerge.bc b/test/loopMerge.bc new file mode 100644 index 0000000000..5d38ccd872 Binary files /dev/null and b/test/loopMerge.bc differ diff --git a/test/mergeBlock.bc b/test/mergeBlock.bc new file mode 100644 index 0000000000..64dc792804 Binary files /dev/null and b/test/mergeBlock.bc differ