[Driver][DirectX] Add /Qembed_debug and /Fd flags by kuilpd · Pull Request #150 · access-softek/llvm-project

kuilpd · 2026-05-29T15:09:10Z

No description provided.

dzhidzhoev · 2026-05-31T21:27:40Z

I have one test failure on Windows with this patch:

******************** TEST 'LLVM :: tools/dxil-dis/shuffle.ll' FAILED ********************
Exit Code: 2

Command Output (stdout):
--
# RUN: at line 1
d:\llvm\build\native\bin\llc.exe -start-after=dxil-flatten-arrays --filetype=obj D:\llvm\llvm-project\llvm\test\tools\dxil-dis\shuffle.ll -o - 2>&1 | d:\llvm\build\native\bin\dxil-dis.exe -o - | d:\llvm\build\native\bin\filecheck.exe D:\llvm\llvm-project\llvm\test\tools\dxil-dis\shuffle.ll
# executed command: 'd:\llvm\build\native\bin\llc.exe' -start-after=dxil-flatten-arrays --filetype=obj 'D:\llvm\llvm-project\llvm\test\tools\dxil-dis\shuffle.ll' -o -
# note: command had no output on stdout or stderr
# error: command failed with exit status: 0xc000001d
# executed command: 'd:\llvm\build\native\bin\dxil-dis.exe' -o -
# .---command stderr------------
# | d:\llvm\build\native\bin\dxil-dis.exe: error: Invalid bitcode size
# `-----------------------------
# error: command failed with exit status: 1
# executed command: 'd:\llvm\build\native\bin\filecheck.exe' 'D:\llvm\llvm-project\llvm\test\tools\dxil-dis\shuffle.ll'
# .---command stderr------------
# | FileCheck error: '<stdin>' is empty.
# | FileCheck command line:  d:\llvm\build\native\bin\filecheck.exe D:\llvm\llvm-project\llvm\test\tools\dxil-dis\shuffle.ll
# `-----------------------------
# error: command failed with exit status: 2

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
********************
Failed Tests (1):
  LLVM :: tools/dxil-dis/shuffle.ll

dzhidzhoev · 2026-05-31T21:31:36Z

+extern cl::opt<bool> EmbedDebug;
+extern cl::opt<std::string> PdbDebugPath;
+


Do we actually need these lines?

No, I forgot to delete these when I experimented with checking these options in this file.

dzhidzhoev · 2026-05-31T23:01:49Z

@@ -1,17 +0,0 @@
-RUN: llc %S/Inputs/SourceInfo.ll -dx-Zss --filetype=obj -o %t.dxbc


Why is this test getting removed?

There is no output to PDB by default anymore, this exact same behavior is tested in test/CodeGen/DirectX/ContainerData/DebugName-user-directory.test now.

We will need it for https://github.com/access-softek/llvm-project/pull/150/changes#r3336804179.

dzhidzhoev · 2026-05-31T23:05:04Z

+  if (!EmbedDebug && SectionName == "ILDB")
+    return true;


More sections need to be filtered out (such as VERS). Please refer to the PDB doc.

VERS is still output there when compiled as a library (which we do in all tests), I'm not sure how to check for this or if we even need to. The other sections are dependent on other flags, as I understand.

Fair point about VERS.
Yet SRCI should be always filtered out here.

dzhidzhoev · 2026-05-31T23:08:46Z

  Globals.emplace_back(
      buildContainerGlobal(M, ModuleConstant, "dx.hash", "HASH"));

  // Emit ILDN part in debug info mode.


We gotta move this comment closer to

mcdxbc::DebugName DebugName; DebugName.setFilename(DebugNameStr); SmallString<64> ILDNData; raw_svector_ostream OS(ILDNData); DebugName.write(OS); addSection(M, Globals, ILDNData, "dx.ildn", "ILDN");

dzhidzhoev · 2026-05-31T23:11:45Z

+  if (!EmbedDebug && PdbDebugPath.empty())
+    return;


ILDN (Shader debug name) part must be emitted when debug info is emitted, regardless of whether /Zi or /Zs is was used to trigger that.
I would just assume that if we have a DICompileUnit in module, then the module is produced by clang in debug info mode, which was triggered by passing /Zi or /Zs (as the main flags triggering debug info emission). And, in that case, debug name part should be created.

I wasn't sure where to put this yet, since we don't have a /Zs flag yet. This patch implements the logic for existing flags, so this can be changed as needed with /Zs patch.

Yes, we only have /Zi, as for now.
ILDN must be emitted regardless of whether /Qembed_debug is passed, and regardless of whether /Fd is passed.
Can we simply emit it based on non-emptiness of M.debug_compile_units()? It's the indicator of whether /Zi was passed to clang-dxc.

dzhidzhoev · 2026-05-31T23:35:36Z

+    if (HasDebugInfo && !EmbedDebug && PdbDebugPath.empty())
+      EmbedDebug = true;


Modifying a cl::opt does not sound like a good idea to me. These variables are shared with other passes, and they are meant to represent user input. I couldn't find many examples where a pass modifies an option, other than in SampleProfileLoader pass initialization.

There is the same check in the driver as well, if /Qembed_debug is not specified with /Zi present, I enable it for llc with -dx-embed-debug anyway, but it does write the warning (same as DXC does). So this check in the backend can only pass when llc is called directly, so I did this just to have the same default behavior. I can remove this, we'll just have to specify the option manually in the tests.

I believe we can get rid of EmbedDebug usage here and in DXContainerGlobals. Thus, it will only affect MCDXContainerWriter. Then, EmbedDebug can be switched on, when needed, in MCDXContainerWriter. And we won't have to write something to cl::opt from a pass.

dzhidzhoev · 2026-05-31T23:39:39Z

    SmallVector<GlobalValue *, 2> Globals;
-    if (HasDebugInfo) {
+    if (HasDebugInfo && (EmbedDebug || !PdbDebugPath.empty())) {


Consider simlpifying this check, so that we don't have duplicate complex if conditions. Maybe, if (!ILDBData.empty()) will be enough?

dzhidzhoev · 2026-05-31T23:50:28Z

+      // If we don't want debug info, strip it from DXIL.
+      StripDebugInfo(M);


StripDebugInfo is an expensive operation (scans all instructions etc). Do we really need to run it if M.debug_compile_units() is empty? Asking since HasDebugInfo argument has been removed.

For example, DebugInfo generation on other targets (DwarfDebug or CodeViewDebug) does not run at all if there are no compile units in a module. See the if condition in DebugHandlerBase::beginModule():

void DebugHandlerBase::beginModule(Module *M) { if (M->debug_compile_units().empty()) Asm = nullptr; else ... }

I believe we can correctly assume that, without DICompileUnits, no debug info is present at all. Otherwise, LLVM IR is not fully correct (if it has debug info without DICompileUnit, which is normally the root of debug information entries tree).

If it is still necessary to do that, let's make a separate commit which removes extra arguments from writeModule, or refactors the code of it in another way.

I joined the 2 arguments because they were the same when called. If HasDebugInfo was true, IsDebug was passed as true; if HasDebugInfo was false, IsDebug was passed as false. Basically, WriteDebug now implies that there is debug info present.

dzhidzhoev · 2026-06-01T00:02:26Z

 // RUN: %clang_dxc -Tlib_6_7 -### -g %s 2>&1 | FileCheck %s --check-prefix=CHECK,CHECK-CMD
 // RUN: %clang_dxc -Tlib_6_7 -### /Zi %s 2>&1 | FileCheck %s
 // RUN: %clang_dxc -Tlib_6_7 -### /Zi /Qembed_debug %s 2>&1 | FileCheck %s
 // RUN: %clang_dxc -Tlib_6_7 -### -Zi %s 2>&1 | FileCheck %s
 // RUN: %clang_dxc -Tlib_6_7 -### -Zi -Qembed_debug %s 2>&1 | FileCheck %s
 // RUN: %clang_dxc -Tlib_6_7 -### -Zi -gcodeview %s 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK-CV
 // RUN: %clang_dxc -Tlib_6_7 -### -Zi -gdwarf %s 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK-DWARF
 // RUN: %clang_dxc -Tlib_6_7 -### -gcodeview -Zi %s 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK-CV
 // RUN: %clang_dxc -Tlib_6_7 -### -gdwarf -Zi %s 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK-DWARF


Let's add some checks for command lines created by Driver in presence of /Fd and/or /Qembed_debug. Either here on in a separate test.

dzhidzhoev · 2026-06-01T00:17:05Z


-static cl::opt<std::string>
-    PdbFileName("dx-pdb-file",
-                cl::desc("Specify the PDB output file path for DirectX target"),
-                cl::value_desc("filename"));
-static cl::opt<std::string> PdbOutputDir(
-    "dx-pdb-dir",
-    cl::desc("Specify the PDB output directory for DirectX target. The file "
-             "name is derived from the shader hash"),
-    cl::value_desc("directory"));
+extern llvm::cl::opt<bool> EmbedDebug;
+cl::opt<std::string> PdbDebugPath(
+    "dx-Fd",
+    cl::desc("Write debug information to the given file, or automatically "
+             "named file in directory when ending in '\\'"),
+    cl::value_desc("filename"));


@asavonic

/Fd is an option of original DXC to specify output PDB file name.
Its behavior depends on whether a path ends with path separator:

If dxc invocation contains /Fd=something, PDB file named "something" is created.

If it contains /Fd=something\ (argument value ends with \), DXC attempts to create PDB file named <MD5-hash-of-bitcode>.pdb in directory something\ (fstat is not called for the path to determine if it's a directory).

As of my knowledge, llc does not have any existing flag which has similar behavior (changing what we do depending on whether a directory path or a file path is provided as a value for the same argument).
Should we make two options for llc (like --dx-pdb-dir and --dx-pdb-file), with explicit case distinction, and implement the logic for choosing which llc invocation argument is right in Clang-dxc driver, or should we implement just one llc flag (--dx-Fd), and implement the abovementioned DXC logic here in llc?

Can we always generate MD5.pdb file in llc, and let the driver rename it if needed?

I implemented this logic in DXContainerGlobals.cpp:177-185. I chose to do this in the backend so that we would have the same arguments in both llc and the Driver to avoid confusion, and also so that if we needed a check to see if Fd was specified, we wouldn't have to check 2 options all the time.

Can we always generate MD5.pdb file in llc, and let the driver rename it if needed?

Then the driver would need to modify the content of llc's output files, because PDB file name is baked into PDB file and into the main llc output file.

dzhidzhoev · 2026-06-01T00:44:24Z

-    if (HasDebugInfo) {
+    if (HasDebugInfo && (EmbedDebug || !PdbDebugPath.empty())) {


The subcondition (EmbedDebug || !PdbDebugPath.empty()) is equivalent to true at this line, considering this assignment above:

if (HasDebugInfo && !EmbedDebug && PdbDebugPath.empty()) EmbedDebug = true;

Assuming HasDebugInfo is true, and execution reached line 255, the body of the if will always be executed.
I'd suggest we create ILDB regardless of flag presence, and do case distinction at later stages, if it is needed.

Makes sense, will do

[Driver][DirectX] Add /Qembed_debug and /Fd flags

e8d7296

kuilpd requested a review from dzhidzhoev May 29, 2026 15:09

dzhidzhoev reviewed Jun 1, 2026

View reviewed changes

dzhidzhoev requested a review from asavonic June 1, 2026 00:17

dzhidzhoev reviewed Jun 1, 2026

View reviewed changes

dzhidzhoev mentioned this pull request Jun 2, 2026

[DirectX] Write DXIL with debug info to ILDB part #158

Open

		extern cl::opt<bool> EmbedDebug;
		extern cl::opt<std::string> PdbDebugPath;

		@@ -1,17 +0,0 @@
		RUN: llc %S/Inputs/SourceInfo.ll -dx-Zss --filetype=obj -o %t.dxbc

		if (HasDebugInfo && !EmbedDebug && PdbDebugPath.empty())
		EmbedDebug = true;

		// If we don't want debug info, strip it from DXIL.
		StripDebugInfo(M);

		if (HasDebugInfo) {
		if (HasDebugInfo && (EmbedDebug \|\| !PdbDebugPath.empty())) {

Conversation

kuilpd commented May 29, 2026

Uh oh!

dzhidzhoev commented May 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dzhidzhoev May 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dzhidzhoev May 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dzhidzhoev Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dzhidzhoev May 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dzhidzhoev Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dzhidzhoev Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

dzhidzhoev commented May 31, 2026 •

edited

Loading

dzhidzhoev May 31, 2026 •

edited

Loading

dzhidzhoev May 31, 2026 •

edited

Loading

dzhidzhoev Jun 1, 2026 •

edited

Loading

dzhidzhoev May 31, 2026 •

edited

Loading

dzhidzhoev Jun 1, 2026 •

edited

Loading

dzhidzhoev Jun 1, 2026 •

edited

Loading