[llvm] [SPIRV] Emitting DebugSource, DebugCompileUnit (PR #97558)
Vyacheslav Levytskyy via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 10 04:19:10 PDT 2024
VyacheslavLevytskyy wrote:
> @michalpaszkowski @VyacheslavLevytskyy Lit test is added. The `SPIRVEmitNonSemanticDI` is the last machine pass before `SPIRVModuleAnalysis` (there are no other passes after) and it's not self contained. It's dependent from what's `SPIRVModuleAnalysis` is doing during during it's analytics pass. I think that to assess it's correctness is necessary to check if after `SPIRVAsmPrinter` instruction emission.
>
> @VyacheslavLevytskyy A lot of tests inside the [Khronos Translator](https://github.com/KhronosGroup/SPIRV-LLVM-Translator) is using SPT (internal, textual, used only for tests) format and/or are mostly covering [OpenCL.DebugInfo.100](https://registry.khronos.org/SPIR-V/specs/unified1/OpenCL.DebugInfo.100.html#_introduction) standard which we aren't. Some mixing NonSemantic.DI and OpenCL.DI in the same test file (by separate RUN). Finding which tests are useful for us, translating them, is maybe worth an effort but the risk (of time investment) to reward (finding useful tests) ratio have, in my opinion, question mark when we compare it to just creating new set of tests. In addition to that - especially at the beginning of development there is a lot of cases which aren't covered and with broken tests (inevitable if we port them) - PR's won't be merged - so we would need to disable a lot of them. At some point it's likely that the risk-reward ratio will change. Nevertheless if someone want's to do it, go through them and add to the Backend useful ones - I'm fully supporting it as a optional side quest for everyone who want to participate in DebugInfo development.
So, two points to think about are, briefly, (1) what volume of tests with which content to target eventually, and (2) what subset of the target test suite it makes sense to introduce now for this initial PR:
(1) As a general approach, I believe that a comprehensive test suite is crucial here, so all must be covered with tests on each stage of development. Talking about contents of test cases, it makes sense to reuse Translator test suite whenever it's possible -- it's cheaper than to start from scratch and has a benefit of don't missing a case.
We have a lot of tests in DebugInfo, and even if there are mixed OpenCL.DebugInfo.100, NonSemantic.Shader.DebugInfo.100 and 200, at the very minimum we still may benefit from picking up subjects of test cases, and I should think from contents as well. I read in https://github.com/KhronosGroup/SPIRV-Registry/blob/main/nonsemantic/NonSemantic.Shader.DebugInfo.100.asciidoc that `The design guidelines for these instructions are: Similarity with OpenCL.DebugInfo.100 ...` and then see a short `list of changes to the OpenCL.DebugInfo.100 specification`, so maybe we can be a bit optimistic about possibilities that Translator's test suite grants us.
As a random example, I've just picked up `test/DebugInfo/DebugInfoNoneEntity.ll` from Translator that we definitely need to show in this initial PR. Another randomly picked up `test/DebugInfo/DebugInfoTypeBasic.ll` contains something already supported and an unsupported piece, so it may be (or may be not) an addition to this PR.
(2) Getting back to the question of how much tests are needed for this PR, I'd say that in my opinion we need to cover at least combinations of values along the following axes:
* different cases of module content (empty, 1 function, many functions), with
* different content of functions (empty function, not empty function), with
* different content of meta-data (present correct debug info, absent debug info, present incorrect and/or partial debug info)
to show translation from llvm ir to spirv code, and
* to demonstrate the contract between the passes, as @michalpaszkowski has suggested.
The reason why I've mentioned Translator earlier is that it may be one more source of inspiration in this context even if you'd like to avoid investing too much time into Translator's test cases at this moment of development of debug info.
https://github.com/llvm/llvm-project/pull/97558
More information about the llvm-commits
mailing list