Fix DebugTypeFunction #3275
Fix DebugTypeFunction #3275
Conversation
|
Restarting CI with a new HEAD |
There was a problem hiding this comment.
I suggest to split this patch into 2. First is addressing an issue with DebugFunction Type. And another is addressing null translation for function declaration.
@pvelesko please also note, that currently existing tests are failing with the patch, I believe their failure is related due to null handling part of the patch.
| DeclOps[SPIRVDebug::Operand::FunctionDeclaration::NameIdx] = BM->getString(Func->getName().str())->getId(); | ||
| DeclOps[SPIRVDebug::Operand::FunctionDeclaration::TypeIdx] = transDbgEntry(Func->getType())->getId(); | ||
| DeclOps[SPIRVDebug::Operand::FunctionDeclaration::SourceIdx] = getSource(Func)->getId(); | ||
| DeclOps[SPIRVDebug::Operand::FunctionDeclaration::LineIdx] = Func->getLine(); | ||
| DeclOps[SPIRVDebug::Operand::FunctionDeclaration::ColumnIdx] = 0; // This version of DISubprogram has no column number |
There was a problem hiding this comment.
nit: we are already using namespace SPIRVDebug::Operand::Function few lines above, so it's not really needed to specify it here.
|
@MrSidims I now realize that I ran the unit tests incorrectly on my machine and that When translating LLVM IR to SPIR-V, functions without explicit declarations need a value for their Declaration operand in the DebugFunction instruction. We use DebugInfoNone, but spirv-val rejects this. LLVM IR: Functions can exist without separate declaration metadata Main problem: The issue is fundamentally a mismatch between LLVM's flexible debug model and SPIR-V's strict validation requirements. @MrSidims I guess I can strip debugInfo for when I am validating SPIR-V in my application but I'm not sure how to properly fix this discrepancy. Any ideas? |
|
@pvelesko IMHO current DebugInfo specification is too strict in terms of forbidding DebugInfoNone usages. I still have a homework to do to lift few of them off in KhronosGroup/SPIRV-Registry#186 and may be try to push for changes in .100 version. In the translator, as you have noticed, the rules are relaxed, may be even too relaxed, which is not a good thing as well. So, about this specifically, I think we should be somewhat leaning not even to LLVM IR debug information specification, but to DWARF, when defining SPIR-V debug info. And unless I'm misinterpreting it: DW_AT_declaration attribute can be unspecified, especially in cases if a debug entity has DW_AT_specification attribute or DW_AT_abstract_origin (at least such interpretation makes sense to me, as why do we need to keep declaration attribute, when we have definition?). I'll do a bit of more research and will open an issue in SPIR-V registry, proposing to relax rule(s) for DebugInfoNone (I could imagine relaxing them, for example, for mem2reged variables), but I can't promise to do it soon. Alternatively (if you agree with the statement, that declaration can be DebugInfoNone) you may open the issue yourself :) |
|
@pvelesko hi, sorry it took a bit longer, but eventually I'm returning to DebugInfoNone placement in SPIR-V. And for this very case it seem to be a straight-forward bug in llvm-spirv. Per both https://registry.khronos.org/SPIR-V/specs/unified1/OpenCL.DebugInfo.100.html#DebugFunction and https://github.com/KhronosGroup/SPIRV-Registry/blob/main/nonsemantic/NonSemantic.Shader.DebugInfo.100.asciidoc#DebugFunction - Declaration operand is optional. Hence the translator mustn't put DebugInfoNone as the last operand in case of declaration's absence. |
|
Created #3354 |
In case of missing declaration we shouldn't place DebugInfoNone for OpenCL and Shader.DebugInfo.100 instruction sets. Solves issue from #3275 Signed-off-by: Sidorov, Dmitry <[email protected]>
In case of missing declaration we shouldn't place DebugInfoNone for OpenCL and Shader.DebugInfo.100 instruction sets. Solves issue from KhronosGroup/SPIRV-LLVM-Translator#3275 Signed-off-by: Sidorov, Dmitry <[email protected]> Original commit: KhronosGroup/SPIRV-LLVM-Translator@fa9bc14efcd68e0
The translator was incorrectly generating SPIR-V debug information for functions. Specifically:
The PR implements two main fixes:
1. Proper DebugFunctionDeclaration Creation (transDbgFunction)
2. Proper Void Debug Type Handling (getVoidDebugType and transDbgSubroutineType)
Added a test and validated that test fails wtihout the fix and passes with it.