[DirectX][HLSL] Add /Qsource_in_debug_module to llc and Clang DXC driver#157
[DirectX][HLSL] Add /Qsource_in_debug_module to llc and Clang DXC driver#157dzhidzhoev wants to merge 1 commit into
Conversation
dx.source metadata nodes contain shader source code information, such as source files content, macro definitions used to compile shader, and compiler arguments. PR for Clang to produce them is currently on review llvm/llvm-project#199689. Original DXC has two modes to handle these nodes: 1. If /Qsource_in_debug_module is not specified, Source Info part (SRCI) is created inside shader's debug PDB file, and dx.source metadata nodes are replaced with empty nodes. 2. If /Qsource_in_debug_module is specified, dx.source nodes are preserved in debug bitcode module, for debug info consumers to use them directly. This patch reimplements the behavior described in item 2. /Qsource_in_debug_module is added to Clang DXC driver, and the corresponding flag is added to llc. In DXILWriterPass, this flag is used to determine how dx.source metadata nodes should be handled.
| def dxc_source_in_debug_module | ||
| : Option<["/", "-"], "Qsource_in_debug_module", KIND_FLAG>, | ||
| Group<dxc_Group>, | ||
| Visibility<[DXCOption]>, | ||
| HelpText<"Embed source code into debug module on DirectX target">; |
There was a problem hiding this comment.
You can shorten this by using class DXCFlag like other flags do.
|
|
||
| if (Args.hasArg(options::OPT_dxc_source_in_debug_module)) { | ||
| CmdArgs.push_back("-mllvm"); | ||
| CmdArgs.push_back("--dx-source-in-debug-module"); |
There was a problem hiding this comment.
Should we use a single dash for all llc options, just for the sake of consistency? I think most of the options are used like that.
There was a problem hiding this comment.
Should we use a single dash for all llc options, just for the sake of consistency?
AFAIK most of llc options (which are rather target-specific or pass-specific) use double dash (that's what I see in llc --help output):
PS D:\llvm\build\native> bin\llc --help | grep -v -- '--' | grep -c '\-\S' (estimate the number of lines with single dash options)
32
PS D:\llvm\build\native> bin\llc --help | grep -c '\--'
219
There was a problem hiding this comment.
Hm, I was judging by how they are usually forwarded in Clang.cpp, no idea why is it like that.
| // fail the Module Verifier if performed in an earlier pass | ||
| legalizeLifetimeIntrinsics(M); | ||
|
|
||
| // Replace dx.source metadata nodes with stubs. |
There was a problem hiding this comment.
Is this meant to be merged with the current upstream? I'm guessing it needs backend PDB functionality to fully implement, shouldn't it be added later?
There was a problem hiding this comment.
It does not strictly require PDB emission functionality.
It would be nice to merge this on top of ILDB part emission patch, but it's not on review as for now. I can postpone sending this PR to LLVM repo if needed.
asavonic
left a comment
There was a problem hiding this comment.
Original DXC has two modes to handle these nodes:
If /Qsource_in_debug_module is not specified, Source Info part (SRCI) is created inside shader's debug PDB file, and dx.source metadata nodes are replaced with empty nodes.
If /Qsource_in_debug_module is specified, dx.source nodes are preserved in debug bitcode module, for debug info consumers to use them directly.
This patch reimplements the behavior described in item 2.
I suppose it also implements item 1 here: clears dx.* debug metadata, but doesn't generate SRCI yet?
LGTM, thank you!
Thank you, I will fix the commit message! |
dx.source metadata nodes contain shader source code information, such as source files content, macro definitions used to compile shader, and compiler arguments.
PR for Clang to produce them is currently on review llvm/llvm-project#199689.
Original DXC has two modes to handle these nodes:
This patch reimplements the behavior described in item 2, and partially reimplements item 1 behavior (SRCI part is not emitted yet).
/Qsource_in_debug_module is added to Clang DXC driver, and the corresponding flag is added to llc. In DXILWriterPass, this flag is used to determine how dx.source metadata nodes should be handled.