BeeEngine/glslang-zig
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Sanitize debug source location tracking for implicit branch and return

Browse source 
This patch tries to attach debug location of a branch/return instruction to its predecessor or the closing brace. If none could be found, no debug info should be emitted.
This commit is contained in:
Qingyuan Zheng 2024-08-26 03:53:07 +00:00 • committed by arcady-lunarg
parent b1fac200c4
commit a496a34b43
30 changed files with 4713 additions and 4253 deletions
Download patch file Download diff file Expand all files Collapse all files

26
SPIRV/GlslangToSpv.cpp
View file

@ -1558,7 +1558,7 @@ TGlslangToSpvTraverser::TGlslangToSpvTraverser(unsigned int spvVersion,
else {
builder.setEmitSpirvDebugInfo();
}
builder.setDebugSourceFile(glslangIntermediate->getSourceFile());
builder.setDebugMainSourceFile(glslangIntermediate->getSourceFile());
// Set the source shader's text. If for SPV version 1.0, include
// a preamble in comments stating the OpModuleProcessed instructions.
@ -2967,6 +2967,12 @@ bool TGlslangToSpvTraverser::visitAggregate(glslang::TVisit visit, glslang::TInt
currentFunction->setDebugLineInfo(sourceFileId, loc.line, loc.column);
}
} else {
if (options.generateDebugInfo) {
if (glslangIntermediate->getSource() == glslang::EShSourceGlsl && node->getSequence().size() > 1) {
auto endLoc = node->getSequence()[1]->getAsAggregate()->getEndLoc();
builder.setDebugSourceLocation(endLoc.line, endLoc.getFilename());
}
}
if (inEntryPoint)
entryPointTerminated = true;
builder.leaveFunction();
@ -4120,7 +4126,7 @@ bool TGlslangToSpvTraverser::visitSwitch(glslang::TVisit /* visit */, glslang::T
if (codeSegments[s])
codeSegments[s]->traverse(this);
else
builder.addSwitchBreak();
builder.addSwitchBreak(true);
}
breakForLoop.pop();
@ -4144,7 +4150,7 @@ void TGlslangToSpvTraverser::visitConstantUnion(glslang::TIntermConstantUnion* n
bool TGlslangToSpvTraverser::visitLoop(glslang::TVisit /* visit */, glslang::TIntermLoop* node)
{
auto blocks = builder.makeNewLoop();
builder.createBranch(&blocks.head);
builder.createBranch(true, &blocks.head);
// Loop control:
std::vector<unsigned int> operands;
@ -4161,7 +4167,7 @@ bool TGlslangToSpvTraverser::visitLoop(glslang::TVisit /* visit */, glslang::TIn
builder.createLoopMerge(&blocks.merge, &blocks.continue_target, control, operands);
if (node->testFirst() && node->getTest()) {
spv::Block& test = builder.makeNewBlock();
builder.createBranch(&test);
builder.createBranch(true, &test);
builder.setBuildPoint(&test);
node->getTest()->traverse(this);
@ -4172,22 +4178,22 @@ bool TGlslangToSpvTraverser::visitLoop(glslang::TVisit /* visit */, glslang::TIn
breakForLoop.push(true);
if (node->getBody())
node->getBody()->traverse(this);
builder.createBranch(&blocks.continue_target);
builder.createBranch(true, &blocks.continue_target);
breakForLoop.pop();
builder.setBuildPoint(&blocks.continue_target);
if (node->getTerminal())
node->getTerminal()->traverse(this);
builder.createBranch(&blocks.head);
builder.createBranch(true, &blocks.head);
} else {
builder.setDebugSourceLocation(node->getLoc().line, node->getLoc().getFilename());
builder.createBranch(&blocks.body);
builder.createBranch(true, &blocks.body);
breakForLoop.push(true);
builder.setBuildPoint(&blocks.body);
if (node->getBody())
node->getBody()->traverse(this);
builder.createBranch(&blocks.continue_target);
builder.createBranch(true, &blocks.continue_target);
breakForLoop.pop();
builder.setBuildPoint(&blocks.continue_target);
@ -4202,7 +4208,7 @@ bool TGlslangToSpvTraverser::visitLoop(glslang::TVisit /* visit */, glslang::TIn
// TODO: unless there was a break/return/discard instruction
// somewhere in the body, this is an infinite loop, so we should
// issue a warning.
builder.createBranch(&blocks.head);
builder.createBranch(true, &blocks.head);
}
}
builder.setBuildPoint(&blocks.merge);
@ -4238,7 +4244,7 @@ bool TGlslangToSpvTraverser::visitBranch(glslang::TVisit /* visit */, glslang::T
if (breakForLoop.top())
builder.createLoopExit();
else
builder.addSwitchBreak();
builder.addSwitchBreak(false);
break;
case glslang::EOpContinue:
builder.createLoopContinue();

36
SPIRV/SpvBuilder.cpp
View file

@ -2182,6 +2182,10 @@ void Builder::addInstruction(std::unique_ptr<Instruction> inst) {
buildPoint->addInstruction(std::move(inst));
}
void Builder::addInstructionNoDebugInfo(std::unique_ptr<Instruction> inst) {
buildPoint->addInstruction(std::move(inst));
}
// Comments in header
Function* Builder::makeEntryPoint(const char* entryPoint)
{
@ -3659,6 +3663,7 @@ Builder::If::If(Id cond, unsigned int ctrl, Builder& gb) :
// Save the current block, so that we can add in the flow control split when
// makeEndIf is called.
headerBlock = builder.getBuildPoint();
builder.createSelectionMerge(mergeBlock, control);
function->addBlock(thenBlock);
builder.setBuildPoint(thenBlock);
@ -3668,7 +3673,7 @@ Builder::If::If(Id cond, unsigned int ctrl, Builder& gb) :
void Builder::If::makeBeginElse()
{
// Close out the "then" by having it jump to the mergeBlock
builder.createBranch(mergeBlock);
builder.createBranch(true, mergeBlock);
// Make the first else block and add it to the function
elseBlock = new Block(builder.getUniqueId(), *function);
@ -3682,11 +3687,10 @@ void Builder::If::makeBeginElse()
void Builder::If::makeEndIf()
{
// jump to the merge block
builder.createBranch(mergeBlock);
builder.createBranch(true, mergeBlock);
// Go back to the headerBlock and make the flow control split
builder.setBuildPoint(headerBlock);
builder.createSelectionMerge(mergeBlock, control);
if (elseBlock)
builder.createConditionalBranch(condition, thenBlock, elseBlock);
else
@ -3732,10 +3736,10 @@ void Builder::makeSwitch(Id selector, unsigned int control, int numSegments, con
}
// Comments in header
void Builder::addSwitchBreak()
void Builder::addSwitchBreak(bool implicit)
{
// branch to the top of the merge block stack
createBranch(switchMerges.top());
createBranch(implicit, switchMerges.top());
createAndSetNoPredecessorBlock("post-switch-break");
}
@ -3746,7 +3750,7 @@ void Builder::nextSwitchSegment(std::vector<Block*>& segmentBlock, int nextSegme
if (lastSegment >= 0) {
// Close out previous segment by jumping, if necessary, to next segment
if (! buildPoint->isTerminated())
createBranch(segmentBlock[nextSegment]);
createBranch(true, segmentBlock[nextSegment]);
}
Block* block = segmentBlock[nextSegment];
block->getParent().addBlock(block);
@ -3758,7 +3762,7 @@ void Builder::endSwitch(std::vector<Block*>& /*segmentBlock*/)
{
// Close out previous segment by jumping, if necessary, to next segment
if (! buildPoint->isTerminated())
addSwitchBreak();
addSwitchBreak(true);
switchMerges.top()->getParent().addBlock(switchMerges.top());
setBuildPoint(switchMerges.top());
@ -3791,14 +3795,14 @@ Builder::LoopBlocks& Builder::makeNewLoop()
void Builder::createLoopContinue()
{
createBranch(&loops.top().continue_target);
createBranch(false, &loops.top().continue_target);
// Set up a block for dead code.
createAndSetNoPredecessorBlock("post-loop-continue");
}
void Builder::createLoopExit()
{
createBranch(&loops.top().merge);
createBranch(false, &loops.top().merge);
// Set up a block for dead code.
createAndSetNoPredecessorBlock("post-loop-break");
}
@ -4240,11 +4244,16 @@ void Builder::createAndSetNoPredecessorBlock(const char* /*name*/)
}
// Comments in header
void Builder::createBranch(Block* block)
void Builder::createBranch(bool implicit, Block* block)
{
Instruction* branch = new Instruction(OpBranch);
branch->addIdOperand(block->getId());
addInstruction(std::unique_ptr<Instruction>(branch));
if (implicit) {
addInstructionNoDebugInfo(std::unique_ptr<Instruction>(branch));
}
else {
addInstruction(std::unique_ptr<Instruction>(branch));
}
block->addPredecessor(buildPoint);
}
@ -4277,7 +4286,10 @@ void Builder::createConditionalBranch(Id condition, Block* thenBlock, Block* els
branch->addIdOperand(condition);
branch->addIdOperand(thenBlock->getId());
branch->addIdOperand(elseBlock->getId());
addInstruction(std::unique_ptr<Instruction>(branch));
// A conditional branch is always attached to a condition expression
addInstructionNoDebugInfo(std::unique_ptr<Instruction>(branch));
thenBlock->addPredecessor(buildPoint);
elseBlock->addPredecessor(buildPoint);
}

16
SPIRV/SpvBuilder.h
View file

@ -108,7 +108,7 @@ public:
spv::Id getMainFileId() const { return mainFileId; }
// Initialize the main source file name
void setDebugSourceFile(const std::string& file)
void setDebugMainSourceFile(const std::string& file)
{
if (trackDebugInfo) {
dirtyLineTracker = true;
@ -420,8 +420,7 @@ public:
// Also reset current last DebugScope and current source line to unknown
void setBuildPoint(Block* bp) {
buildPoint = bp;
// TODO: Technically, change of build point should set line tracker dirty. But we'll have bad line info for
// branch instructions. Commenting this for now because at least this matches the old behavior.
dirtyLineTracker = true;
dirtyScopeTracker = true;
}
Block* getBuildPoint() const { return buildPoint; }
@ -430,6 +429,11 @@ public:
// Optionally, additional debug info instructions may also be prepended.
void addInstruction(std::unique_ptr<Instruction> inst);
// Append an instruction to the end of the current build point without prepending any debug instructions.
// This is useful for insertion of some debug info instructions themselves or some control flow instructions
// that are attached to its predecessor instruction.
void addInstructionNoDebugInfo(std::unique_ptr<Instruction> inst);
// Make the entry-point function. The returned pointer is only valid
// for the lifetime of this builder.
Function* makeEntryPoint(const char*);
@ -643,7 +647,7 @@ public:
const std::vector<int>& valueToSegment, int defaultSegment, std::vector<Block*>& segmentBB);
// Add a branch to the innermost switch's merge block.
void addSwitchBreak();
void addSwitchBreak(bool implicit);
// Move to the next code segment, passing in the return argument in makeSwitch()
void nextSwitchSegment(std::vector<Block*>& segmentBB, int segment);
@ -865,7 +869,9 @@ public:
void dump(std::vector<unsigned int>&) const;
void createBranch(Block* block);
// Add a branch to the target block.
// If set implicit, the branch instruction shouldn't have debug source location.
void createBranch(bool implicit, Block* block);
void createConditionalBranch(Id condition, Block* thenBlock, Block* elseBlock);
void createLoopMerge(Block* mergeBlock, Block* continueBlock, unsigned int control,
const std::vector<unsigned int>& operands);