[llvm] [SPIR-V] Emit DebugTypePointer from NonSemantic DI (PR #109287)

via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 4 03:24:46 PDT 2024


================
@@ -0,0 +1,280 @@
+; RUN: llc --verify-machineinstrs --spv-emit-nonsemantic-debug-info --spirv-ext=+SPV_KHR_non_semantic_info --print-after=spirv-nonsemantic-debug-info -O0 -mtriple=spirv64-unknown-unknown %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-MIR
+; RUN: llc --verify-machineinstrs --spv-emit-nonsemantic-debug-info --spirv-ext=+SPV_KHR_non_semantic_info -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s --check-prefix=CHECK-SPIRV
+; RUN: llc --verify-machineinstrs -O0 -mtriple=spirv64-unknown-unknown --spirv-ext=+SPV_KHR_non_semantic_info %s -o - | FileCheck %s --check-prefix=CHECK-OPTION
+; When type is void * the spirv-val incorrectly raises an error when DebugInfoNone is set as <id> Base Type argument of DebugTypePointer.   
----------------
bwlodarcz wrote:

> I don't see a way to use `DebugInfoNone` as `<id> Base Type` argument of `DebugTypePointer`. In the specification we read `Base Type is <id> of debugging instruction which represents the pointee type.`. There is no explicit statement in the description of the Instruction that Id may be substituted by DebugInfoNone.
> 
> Please see [KhronosGroup/SPIRV-Registry#280 (comment)](https://github.com/KhronosGroup/SPIRV-Registry/issues/280#issuecomment-2346219612) for detailed comments from the author of the specification and [#106980 (comment)](https://github.com/llvm/llvm-project/pull/106980#discussion_r1756908439) for a similar discussion in one of previous PR's.
> 
> I don't object against proceeding with this PR as is, given that this issue will be addressed in the next PR. Please change the description of the issue above from "... spirv-val incorrectly raises an error ..." to a brief TODO statement, so that the test case holds that it's a TODO.

I don't see any possible sensible solution to avoid it.
The problem is that's basically impossible to handle `void *` scenario if we don't utilize `DebugInfoNone`. The `void *` is basically a type erasure in C and C++ and just plainly doesn't have `BaseType`.
If we want to create composite Null BaseType then what BitLength or StorageClass should it have?
Isn't it exactly the purpose of DebugInfoNone to be used in a cases like this?

>DebugInfoNone

>Other instructions can refer to this one in case the debugging information is unknown, not available, or not applicable.

Matches exact definition of the problem which we have here.
DI info is unknown, not available and even not applicable - all three. 

>From DebugTypeEnum
> Underlying Type is a debugging instruction that describes the underlying type of the enum in the source program. If the underlying type is not specified in the source program, this operand must refer to [DebugInfoNone](https://htmlpreview.github.io/?https://github.com/KhronosGroup/SPIRV-Registry/blob/main/nonsemantic/NonSemantic.Shader.DebugInfo.100.html#DebugInfoNone).

So we have precedence. The only difference is that in DebugTypeEnum we have it described and in this we aren't.
This is a somewhat misguided approach taken by spec. If you have null type then you should create a general rule in which cases it's applicable - if you want to specify every one instruction or argument where it can or can not be needed then you are certainly missing some unforeseen cases and making holes in spec and making divergence between implementations.
Which comes back to the linked [Registry discussion](https://github.com/KhronosGroup/SPIRV-Registry/issues/280#issuecomment-2346219612) and pointed out by @MrSidims. If what the author saying is true  then the spec needs a LOT more default cases and null substitutions to handle.


https://github.com/llvm/llvm-project/pull/109287


More information about the llvm-commits mailing list