diff --git a/mlir/lib/Dialect/SPIRV/Serialization/Deserializer.cpp b/mlir/lib/Dialect/SPIRV/Serialization/Deserializer.cpp --- a/mlir/lib/Dialect/SPIRV/Serialization/Deserializer.cpp +++ b/mlir/lib/Dialect/SPIRV/Serialization/Deserializer.cpp @@ -62,10 +62,12 @@ struct BlockMergeInfo { Block *mergeBlock; Block *continueBlock; // nullptr for spv.selection + Location loc; - BlockMergeInfo() : mergeBlock(nullptr), continueBlock(nullptr) {} - BlockMergeInfo(Block *m, Block *c = nullptr) - : mergeBlock(m), continueBlock(c) {} + BlockMergeInfo(Location location) + : mergeBlock(nullptr), continueBlock(nullptr), loc(location) {} + BlockMergeInfo(Location location, Block *m, Block *c = nullptr) + : mergeBlock(m), continueBlock(c), loc(location) {} }; /// A struct for containing OpLine instruction information. @@ -1539,7 +1541,11 @@ } auto *target = getOrCreateBlock(operands[0]); - opBuilder.create(unknownLoc, target); + auto loc = createFileLineColLoc(opBuilder); + // The preceding instruction for the OpBranch instruction could be an + // OpLoopMerge or an OpSelectionMerge instruction, in this case they will have + // the same OpLine information. + opBuilder.create(loc, target); clearDebugLine(); return success(); @@ -1566,9 +1572,12 @@ if (operands.size() == 5) { weights = std::make_pair(operands[3], operands[4]); } - + // The preceding instruction for the OpBranchConditional instruction could be + // an OpSelectionMerge instruction, in this case they will have the same + // OpLine information. + auto loc = createFileLineColLoc(opBuilder); opBuilder.create( - unknownLoc, condition, trueBlock, + loc, condition, trueBlock, /*trueArguments=*/ArrayRef(), falseBlock, /*falseArguments=*/ArrayRef(), weights); @@ -1616,8 +1625,9 @@ } auto *mergeBlock = getOrCreateBlock(operands[0]); + auto loc = createFileLineColLoc(opBuilder); - if (!blockMergeInfo.try_emplace(curBlock, mergeBlock).second) { + if (!blockMergeInfo.try_emplace(curBlock, loc, mergeBlock).second) { return emitError( unknownLoc, "a block cannot have more than one OpSelectionMerge instruction"); @@ -1643,8 +1653,10 @@ auto *mergeBlock = getOrCreateBlock(operands[0]); auto *continueBlock = getOrCreateBlock(operands[1]); + auto loc = createFileLineColLoc(opBuilder); - if (!blockMergeInfo.try_emplace(curBlock, mergeBlock, continueBlock).second) { + if (!blockMergeInfo.try_emplace(curBlock, loc, mergeBlock, continueBlock) + .second) { return emitError( unknownLoc, "a block cannot have more than one OpLoopMerge instruction"); @@ -1832,7 +1844,6 @@ LLVM_DEBUG(llvm::dbgs() << "[cf] block " << block << " is a function entry block\n"); } - for (auto &op : *block) newBlock->push_back(op.clone(mapper)); } @@ -1913,10 +1924,12 @@ if (Block *mappedTo = mapper.lookupOrNull(newMerge)) newMerge = mappedTo; + // Keep original location for nested selection/loop ops. + Location loc = it->second.loc; // The iterator should be erased before adding a new entry into // blockMergeInfo to avoid iterator invalidation. blockMergeInfo.erase(it); - blockMergeInfo.try_emplace(newHeader, newMerge, newContinue); + blockMergeInfo.try_emplace(newHeader, loc, newMerge, newContinue); } // The structured selection/loop's entry block does not have arguments. @@ -2017,13 +2030,12 @@ << "[cf] continue block " << continueBlock << ":\n"); LLVM_DEBUG(continueBlock->print(llvm::dbgs())); } - // Erase this case before calling into structurizer, who will update // blockMergeInfo. blockMergeInfo.erase(blockMergeInfo.begin()); - if (failed(ControlFlowStructurizer::structurize(unknownLoc, blockMergeInfo, - headerBlock, mergeBlock, - continueBlock))) + if (failed(ControlFlowStructurizer::structurize(mergeInfo.loc, + blockMergeInfo, headerBlock, + mergeBlock, continueBlock))) return failure(); } diff --git a/mlir/lib/Dialect/SPIRV/Serialization/Serializer.cpp b/mlir/lib/Dialect/SPIRV/Serialization/Serializer.cpp --- a/mlir/lib/Dialect/SPIRV/Serialization/Serializer.cpp +++ b/mlir/lib/Dialect/SPIRV/Serialization/Serializer.cpp @@ -391,6 +391,12 @@ /// A flag which indicates if the debuginfo should be emitted. bool emitDebugInfo = false; + /// A flag which indicates if the last processed instruction was a merge + /// instruction. + /// According to SPIR-V spec: "If a branch merge instruction is used, the last + /// OpLine in the block must be before its merge instruction". + bool lastProcessedWasMergeInst = false; + /// The of the OpString instruction, which specifies a file name, for /// use by other debug instructions. uint32_t fileID = 0; @@ -1518,11 +1524,14 @@ auto *headerBlock = selectionOp.getHeaderBlock(); auto *mergeBlock = selectionOp.getMergeBlock(); auto mergeID = getBlockID(mergeBlock); + auto loc = selectionOp.getLoc(); // Emit the selection header block, which dominates all other blocks, first. // We need to emit an OpSelectionMerge instruction before the selection header // block's terminator. auto emitSelectionMerge = [&]() { + emitDebugLine(functionBody, loc); + lastProcessedWasMergeInst = true; // TODO(antiagainst): properly support selection control here encodeInstructionInto( functionBody, spirv::Opcode::OpSelectionMerge, @@ -1567,6 +1576,7 @@ auto headerID = getBlockID(headerBlock); auto continueID = getBlockID(continueBlock); auto mergeID = getBlockID(mergeBlock); + auto loc = loopOp.getLoc(); // This LoopOp is in some MLIR block with preceding and following ops. In the // binary format, it should reside in separate SPIR-V blocks from its @@ -1583,6 +1593,8 @@ // need to emit an OpLoopMerge instruction before the loop header block's // terminator. auto emitLoopMerge = [&]() { + emitDebugLine(functionBody, loc); + lastProcessedWasMergeInst = true; // TODO(antiagainst): properly support loop control here encodeInstructionInto( functionBody, spirv::Opcode::OpLoopMerge, @@ -1622,11 +1634,13 @@ arguments.push_back(val.cast().getInt()); } + emitDebugLine(functionBody, condBranchOp.getLoc()); return encodeInstructionInto(functionBody, spirv::Opcode::OpBranchConditional, arguments); } LogicalResult Serializer::processBranchOp(spirv::BranchOp branchOp) { + emitDebugLine(functionBody, branchOp.getLoc()); return encodeInstructionInto(functionBody, spirv::Opcode::OpBranch, {getOrCreateBlockID(branchOp.getTarget())}); } @@ -1873,6 +1887,11 @@ if (!emitDebugInfo) return success(); + if (lastProcessedWasMergeInst) { + lastProcessedWasMergeInst = false; + return success(); + } + auto fileLoc = loc.dyn_cast(); if (fileLoc) encodeInstructionInto(binary, spirv::Opcode::OpLine, diff --git a/mlir/test/Dialect/SPIRV/Serialization/debug.mlir b/mlir/test/Dialect/SPIRV/Serialization/debug.mlir --- a/mlir/test/Dialect/SPIRV/Serialization/debug.mlir +++ b/mlir/test/Dialect/SPIRV/Serialization/debug.mlir @@ -66,4 +66,81 @@ // CHECK: loc({{".*debug.mlir"}}:67:5) spv.Return } + + spv.func @loop(%count : i32) -> () "None" { + %zero = spv.constant 0: i32 + %one = spv.constant 1: i32 + %ivar = spv.Variable init(%zero) : !spv.ptr + %jvar = spv.Variable init(%zero) : !spv.ptr + spv.loop { + // CHECK: loc({{".*debug.mlir"}}:75:5) + spv.Branch ^header + ^header: + %ival0 = spv.Load "Function" %ivar : i32 + %icmp = spv.SLessThan %ival0, %count : i32 + // CHECK: loc({{".*debug.mlir"}}:75:5) + spv.BranchConditional %icmp, ^body, ^merge + ^body: + spv.Store "Function" %jvar, %zero : i32 + spv.loop { + // CHECK: loc({{".*debug.mlir"}}:85:7) + spv.Branch ^header + ^header: + %jval0 = spv.Load "Function" %jvar : i32 + %jcmp = spv.SLessThan %jval0, %count : i32 + // CHECK: loc({{".*debug.mlir"}}:85:7) + spv.BranchConditional %jcmp, ^body, ^merge + ^body: + // CHECK: loc({{".*debug.mlir"}}:95:9) + spv.Branch ^continue + ^continue: + %jval1 = spv.Load "Function" %jvar : i32 + %add = spv.IAdd %jval1, %one : i32 + spv.Store "Function" %jvar, %add : i32 + // CHECK: loc({{".*debug.mlir"}}:101:9) + spv.Branch ^header + ^merge: + // CHECK: loc({{".*debug.mlir"}}:85:7) + spv._merge + // CHECK: loc({{".*debug.mlir"}}:85:7) + } + // CHECK: loc({{".*debug.mlir"}}:108:7) + spv.Branch ^continue + ^continue: + %ival1 = spv.Load "Function" %ivar : i32 + %add = spv.IAdd %ival1, %one : i32 + spv.Store "Function" %ivar, %add : i32 + // CHECK: loc({{".*debug.mlir"}}:114:7) + spv.Branch ^header + ^merge: + // CHECK: loc({{".*debug.mlir"}}:75:5) + spv._merge + // CHECK: loc({{".*debug.mlir"}}:75:5) + } + spv.Return + } + + spv.func @selection(%cond: i1) -> () "None" { + %zero = spv.constant 0: i32 + %one = spv.constant 1: i32 + %two = spv.constant 2: i32 + %var = spv.Variable init(%zero) : !spv.ptr + spv.selection { + // CHECK: loc({{".*debug.mlir"}}:128:5) + spv.BranchConditional %cond [5, 10], ^then, ^else + ^then: + spv.Store "Function" %var, %one : i32 + // CHECK: loc({{".*debug.mlir"}}:134:7) + spv.Branch ^merge + ^else: + spv.Store "Function" %var, %two : i32 + // CHECK: loc({{".*debug.mlir"}}:138:7) + spv.Branch ^merge + ^merge: + // CHECK: loc({{".*debug.mlir"}}:128:5) + spv._merge + // CHECK: loc({{".*debug.mlir"}}:128:5) + } + spv.Return + } }