-
Notifications
You must be signed in to change notification settings - Fork 116
Bugs with handling NonSemanticShaderDebugInfo in SPIRVToLLVMDbgTran.cpp #2904
Description
Summary
Hey all, while I was working on investigating getting debug source correlations from a DXC compiled shader, I noticed a few issues in SPIRVToLLVMDbgTran.cpp when dealing with NonSemantic.Shader.DebugInfo.100.
Since I don't have the test suite, or the experience with this codebase to be confident in my fixes, I figured I would raise them here.
Let me know if this is the right forum, or if I should break up each of these into individual issues! Since they all seem relatively small, I figured I would batch them.
Repro
I've attached a SPIR-V file that should run into all these issues: example_shader.zip
Compiling it with this command line:
amdllpc.exe --auto-layout-desc -o="output.bin" -gfxip=10.3.0 -trim-debug-info=false "example_shader.spv"
1. SPIRVToLLVMDbgTran::transTypeComposite
llpc/llpc/translator/lib/SPIRV/SPIRVToLLVMDbgTran.cpp
Lines 330 to 331 in 5278a3f
| switch (Ops[TagIdx]) { | |
| case SPIRVDebug::Class: |
I think this should be switch (getConstant(Ops[TagIdx])) when reading NonSemantic debug info?
From the spec:
Tag is the of a 32-bit integer OpConstant with a value from the Composite Types table that specifies the kind of the composite type.
2. DebugTypeMember doesn't have a parent scope with NonSemantic debug info.
DebugTypeMember had its parent scope removed as an argument in NonSemantic debug info.
From the spec:
DebugTypeMember no longer has a parent Scope , this is implicit from which DebugTypeComposite lists it as a member.
As a result, these two lines in SPIRVToLLVMDbgTran::transTypeMember are incorrect.
| assert(Ops.size() >= MinOperandCount && "Invalid number of operands"); |
| DIScope *Scope = getScope(BM->getEntry(Ops[ParentIdx])); |
We don't have access to a parent scope and the number of arguments differs from OpenCL debug info. I'm not sure how best to reference the parent composite type in order to derive the scope here.
3. SPIRVToLLVMDbgTran::transDebugIntrinsic fails on DebugNoLine
This switch:
| switch (DebugInst->getExtOp()) { |
Fails when we encounter a DebugNoLine op since we don't account for it. We end up falling into the llvm_unreachable below.
Perhaps its as simple as adding SPIRVDebug::NoLine below?
llpc/llpc/translator/lib/SPIRV/SPIRVToLLVMDbgTran.cpp
Lines 903 to 904 in 5278a3f
| case SPIRVDebug::Line: | |
| return nullptr; |
4. SPIRVToLLVMDbgTran::transDebugScope references a DebugSource but incorrectly considers it a string constant.
In SPIRVToLLVMDbgTran::transDebugScope we reference a filename from a SPIRVLine.
| Filename = LineInfo->getFileNameStr(); |
But with NonSemantic debug info, the filename isn't a string type. It's actually a DebugSource type.
From the spec:
Source is a previously declared DebugSource indicating the file containing the location.
This causes a crash.
5. Index out of range in SPIRVToLLVMDbgTran::transSource
I think this branch is incorrect:
llpc/llpc/translator/lib/SPIRV/SPIRVToLLVMDbgTran.cpp
Lines 1042 to 1043 in 5278a3f
| if (ops.size() > FileIdx) | |
| source = getString(ops[TextIdx]); |
I think it needs to be if (ops.size() > TextIdx).
This causes a crash.
Cheers!