DirectXShaderCompiler icon indicating copy to clipboard operation
DirectXShaderCompiler copied to clipboard

DXIL debug locations do not respect #line directives

Open maoenpei opened this issue 3 years ago • 6 comments

Hi, I'm working on a profiling tool that requires DXIL -> HLSL line mapping for low-level to high-level counters aggregating. However after dxc validator version >= 1.6, the debug locations from DXIL do not respect #line directives anymore. I think this issue is introduced from https://github.com/microsoft/DirectXShaderCompiler/pull/2991.

The HLSL source is also extracted from DXIL bytecode. When it has "#line" directives in it, we usually process the source and build virtual source files based on the line numbers followed "#line" directives, before we are able to display the HLSL source / DXIL text and the correlation between them on our UIs. However, when the debug locations do not respect "#line" directives, we have to display the unprocessed source on our UI, otherwise the correlation breaks.

At most cases (like UE4 game development), the processed sources (the virtual sources built from the extracted source of DXIL bytecode) are the original source files of developers and what's embedded in DXIL bytecode is a file merged by some tools before passing it to dxc. The change of dxc makes our displayed source less readable. So the virtual sources are more expected to be displayed to our users.

In my humble opinion, a regular compiler should always respect "#line" directives. And we always extract virtual sources for GLSL -> SPIRV (by glslc) and HLSL -> SPIRV (by dxc), so dxc (with version >= 1.6) compiling HLSL to DXIL is the only exception.

What I expect:

  • Restore the default behavior of "#line" locations processing.
  • If some clients of dxc need the locations of unprocessed source, add an option for dxc to do that.

Does this make sense?

maoenpei avatar Aug 24 '22 13:08 maoenpei

Hi! Thank you for the bug report! I need some clarifications:

The HLSL source is also extracted from DXIL bytecode. When it has "#line" directives in it, we usually process the source and build virtual source files based on the line numbers followed "#line" directives,

  1. Do you mean the sources you display in the tool UI is the sources you retrieve from the DXIL?
  2. Do you manually process the source from DXIL into virtual files based on the #line directives?

adam-yang avatar Aug 29 '22 22:08 adam-yang

Hi! Thank you for the bug report! I need some clarifications:

The HLSL source is also extracted from DXIL bytecode. When it has "#line" directives in it, we usually process the source and build virtual source files based on the line numbers followed "#line" directives,

  1. Do you mean the sources you display in the tool UI is the sources you retrieve from the DXIL?
  2. Do you manually process the source from DXIL into virtual files based on the #line directives?

Thanks.

  1. It depends on our user (the programmer of the application). If user provides DXIL bytecode with embedded HLSL source, we need to extract the source and display that (better to display virtual files but with the described issue we have to display the unprocessed source). We also support individual files through specifying pdb search folder in the tool, but that's a different case. (I have not go through the pdb path but I'll be thankful if you could introduce a little bit about the difference.)
  2. Yes, we manually process the source extracted from DXIL into virtual files.

The tool we are developing: https://developer.nvidia.com/nsight-graphics

maoenpei avatar Aug 30 '22 06:08 maoenpei

Thanks for the response.

We intentionally made the debug info to point to files in the PDB instead of their #line directives. Our GPU debugging tool currently always displays the sources from the PDB/DXIL and relies on the debug locations to match. In the past, our debug locations respected the #line directives and the tool had to either construct virtual files, which was unreliable; or locate files on the machine, which was often impossible. That's why we made the change to the current behaviour.

If nsight can reliably process the embedded files into virtual files based on their #line directives, there should be a one-to-one line mapping between them. Couldn't you just map the debug info's locations to the virtual files this way?

adam-yang avatar Aug 31 '22 03:08 adam-yang

If nsight can reliably process the embedded files into virtual files based on their #line directives, there should be a one-to-one line mapping between them. Couldn't you just map the debug info's locations to the virtual files this way?

Yes, it's possible and we considered this as a candidate solution. But this looks like a WAR on another WAR and we need to make totally different code path for DXIL debug info processing. So the first priority is to fix an issue at where the issue appear (in this case the dxc compiler), for 2 main reasons:

  • As that I've mentioned, a regular compiler should always respect "#line" directives. dxc is a powerful and widely used shader compiler, which (IMHO) would best to follow the common behavior.
  • It's more convenient to fix this issue in dxc code and if there are some requirements to change the behavior, an extra option is necessary.

maoenpei avatar Aug 31 '22 05:08 maoenpei

As that I've mentioned, a regular compiler should always respect "#line" directives. dxc is a powerful and widely used shader compiler, which (IMHO) would best to follow the common behavior.

We do ensure the #line directives are respected when it comes to error messages. We consider this the main use-case of #line directives.

It's more convenient to fix this issue in dxc code and if there are some requirements to change the behavior, an extra option is necessary.

We have partner teams whose products depend on the current behaviour. All the teams involved will have to make non-trivial changes to accommodate if #2991 were to be reverted.

The GPU debugging tool that we support only displays the raw source submitted to the compiler. We have been doing it this way for two years and haven't had any complaints from users. If shader authors felt like the merged file was hard to read, they always have the option to submit the unmerged files instead.

Another note: the #line directive is a one-way process. If the debug info uses the #line locations, those locations cannot be translated back to locations in the raw file. Conversely, keeping the raw locations allows tools to process the files and translate debug locations to #line locations if necessary. In my opinion, this is the most flexible solution.

adam-yang avatar Aug 31 '22 21:08 adam-yang

Thanks Adam, I've discussed with my colleagues and agree that the debug information generation is not standardized and no spec (either C++ or HLSL) has detailed descriptions for that. But please know the current behavior differs from MSVC / gcc / glslc, or even dxc's SPIR-V backend. I understand it would be painful to tell all the clients / partners to change their project code that is depending on the current behavior. If we get a chance to design it from scratch, I would highly recommend to follow the behavior of other compilers. But now I'm fine to keep the current behavior (将错就错). I would still suggest (as a "nice to have" feature) to add a compiler flag that will give developers a choice: the default is the current behavior and with that flag dxc will respect #line directives in the debug locations. It's up to you.

maoenpei avatar Sep 05 '22 12:09 maoenpei