Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kuilpd wants to merge 1 commit into
base: dx-debug
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions clang/include/clang/Basic/DiagnosticDriverKinds.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -834,6 +834,16 @@ def err_drv_dxc_missing_target_profile : Error<
"target profile option (-T) is missing">;
def err_drv_dxc_invalid_shader_hash
: Error<"cannot specify both /Zss and /Zsb">;
def err_drv_no_debug_info_for_embed_debug
: Error<"must enable debug info with /Zi for /Qembed_debug">;
def err_drv_no_debug_info_for_Fd
: Error<"/Fd specified, but no Debug Info was found in the shader; "
"please use the /Zi or /Zs switch to generate debug information "
"compiling this shader">;
def warn_drv_dxc_no_output_for_debug
: Warning<"no output provided for debug - embedding PDB in shader "
"container; use /Qembed_debug to silence this warning">,
InGroup<DiagGroup<"default-embed-debug">>;
def err_drv_hlsl_unsupported_target : Error<
"HLSL code generation is unsupported for target '%0'">;
def err_drv_hlsl_bad_shader_required_in_target : Error<
Expand Down
27 changes: 18 additions & 9 deletions clang/include/clang/Options/Options.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9319,10 +9319,9 @@ def _SLASH_ZH_SHA1 : CLFlag<"ZH:SHA1">,
HelpText<"Use SHA1 for file checksums in debug info">,
Alias<gsrc_hash_EQ>, AliasArgs<["sha1"]>;
def _SLASH_ZH_SHA_256 : CLFlag<"ZH:SHA_256">,
HelpText<"Use SHA256 for file checksums in debug info">,
Alias<gsrc_hash_EQ>, AliasArgs<["sha256"]>;
def _SLASH_Zi : CLFlag<"Zi", [CLOption, DXCOption]>, Alias<g_Flag>,
HelpText<"Like /Z7">;
HelpText<"Use SHA256 for file checksums in debug info">,
Alias<gsrc_hash_EQ>,
AliasArgs<["sha256"]>;
def _SLASH_Zp : CLJoined<"Zp">,
HelpText<"Set default maximum struct packing alignment">,
Alias<fpack_struct_EQ>;
Expand Down Expand Up @@ -9652,6 +9651,18 @@ def dxc_Fc : DXCJoinedOrSeparate<"Fc">,
HelpText<"Output assembly listing file">;
def dxc_Frs : DXCJoinedOrSeparate<"Frs">,
HelpText<"Output additional root signature object file">;
def dxc_Fd
: DXCJoinedOrSeparate<"Fd">,
HelpText<"Write debug information to the given file, or automatically "
"named file in directory when ending in '\\'">;
def dxc_Qembed_debug
: DXCFlag<"Qembed_debug">,
HelpText<"Embed PDB in shader container (must be used with /Zi)">;
def dxc_Zi : Option<["/", "-"], "Zi", KIND_FLAG>,
Group<dxc_Group>,
Alias<g_Flag>,
Visibility<[CLOption, DXCOption]>,
HelpText<"Enable debug information">;
def dxc_Zss : DXCFlag<"Zss">,
HelpText<"Compute Shader Hash considering source information">;
def dxc_Zsb : DXCFlag<"Zsb">,
Expand Down Expand Up @@ -9747,11 +9758,9 @@ def dxc_validator_path_EQ : Joined<["--"], "dxv-path=">, Group<dxc_Group>,
HelpText<"DXIL validator installation path">;
def dxc_disable_validation : DXCFlag<"Vd">,
HelpText<"Disable validation">;
def dxc_gis : DXCFlag<"Gis">,
HelpText<"Enable IEEE strict mode (equivalent to -ffp-model=strict)">;
def : Option<["/", "-"], "Qembed_debug", KIND_FLAG>, Group<dxc_Group>,
Flags<[Ignored]>, Visibility<[DXCOption]>,
HelpText<"Embed PDB in shader container (ignored)">;
def dxc_gis
: DXCFlag<"Gis">,
HelpText<"Enable IEEE strict mode (equivalent to -ffp-model=strict)">;
def spirv : DXCFlag<"spirv">,
HelpText<"Generate SPIR-V code">;
def metal : DXCFlag<"metal">, HelpText<"Generate Metal library">;
Expand Down
19 changes: 19 additions & 0 deletions clang/lib/Driver/ToolChains/Clang.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3777,6 +3777,25 @@ static void RenderHLSLOptions(const Driver &D, const ArgList &Args,
CmdArgs.push_back("-mllvm");
CmdArgs.push_back("--dx-source-in-debug-module");
}
bool Zi = Args.hasArg(options::OPT_g_Flag);
bool Qembed_debug = Args.hasArg(options::OPT_dxc_Qembed_debug);
Arg *Fd = Args.getLastArg(options::OPT_dxc_Fd);
if (Zi && !Fd && !Qembed_debug) {
D.Diag(diag::warn_drv_dxc_no_output_for_debug);
Qembed_debug = true;
}
if (Qembed_debug && !Zi)
D.Diag(diag::err_drv_no_debug_info_for_embed_debug);
if (Fd && !Zi)
D.Diag(diag::err_drv_no_debug_info_for_Fd);
if (Qembed_debug) {
CmdArgs.push_back("-mllvm");
CmdArgs.push_back("-dx-embed-debug");
}
if (Fd) {
CmdArgs.push_back("-mllvm");
CmdArgs.push_back(Args.MakeArgString("-dx-Fd=" + Twine(Fd->getValue())));
}
}

static void RenderOpenACCOptions(const Driver &D, const ArgList &Args,
Expand Down
8 changes: 8 additions & 0 deletions clang/test/Driver/dxc_debug.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,11 @@
// Make sure dxc command line arguments are passed to clang invocation.
// CHECK-SAME: -fdx-record-command-line
// CHECK-CMD-SAME: --driver-mode=dxc -T lib_6_7 -### -g {{.*}}dxc_debug.hlsl -S -O3

// Check errors and warnings
// RUN: %clang_dxc -Tlib_6_7 -### /Zi %s 2>&1 | FileCheck %s --check-prefix=WARN-EMBED
// WARN-EMBED: warning: no output provided for debug - embedding PDB in shader container
// RUN: not %clang_dxc -Tlib_6_7 -### /Qembed_debug %s 2>&1 | FileCheck %s --check-prefix=ERROR-NODBG0
// ERROR-NODBG0: error: must enable debug info with /Zi for /Qembed_debug
// RUN: not %clang_dxc -Tlib_6_7 -### /Fd %t.pdb %s 2>&1 | FileCheck %s --check-prefix=ERROR-NODBG1
// ERROR-NODBG1: error: /Fd specified, but no Debug Info was found in the shader
1 change: 1 addition & 0 deletions llvm/include/llvm/MC/MCDXContainerWriter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ class DXContainerObjectWriter final : public MCDXContainerBaseWriter,

protected:
ArrayRef<MCDXContainerPart> getParts() override;
bool shouldSkipSection(StringRef SectionName, size_t SectionSize) override;

public:
DXContainerObjectWriter(std::unique_ptr<MCDXContainerTargetWriter> MOTW,
Expand Down
12 changes: 12 additions & 0 deletions llvm/lib/MC/MCDXContainerWriter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,13 @@
#include "llvm/MC/MCSection.h"
#include "llvm/MC/MCValue.h"
#include "llvm/Support/Alignment.h"
#include "llvm/Support/CommandLine.h"

using namespace llvm;

cl::opt<bool> EmbedDebug("dx-embed-debug",
cl::desc("Embed PDB in shader container"));

MCDXContainerTargetWriter::~MCDXContainerTargetWriter() = default;

MCDXContainerBaseWriter::~MCDXContainerBaseWriter() = default;
Expand Down Expand Up @@ -132,6 +136,14 @@ ArrayRef<MCDXContainerPart> DXContainerObjectWriter::getParts() {
return Parts;
}

bool DXContainerObjectWriter::shouldSkipSection(StringRef SectionName,
size_t SectionSize) {
// Do not write ILDB part if we're not embedding it.
if (!EmbedDebug && SectionName == "ILDB")
return true;
Comment on lines +142 to +143
Copy link
Copy Markdown
Contributor

@dzhidzhoev dzhidzhoev May 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Author

@kuilpd kuilpd Jun 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

@dzhidzhoev dzhidzhoev Jun 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

return MCDXContainerBaseWriter::shouldSkipSection(SectionName, SectionSize);
}

uint64_t DXContainerObjectWriter::writeObject() {
// TODO write only necessary sections.
write(W.OS, getContext().getTargetTriple());
Expand Down
69 changes: 34 additions & 35 deletions llvm/lib/Target/DirectX/DXContainerGlobals.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,12 @@ using namespace llvm;
using namespace llvm::dxil;
using namespace llvm::mcdxbc;

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"));
Comment on lines 41 to +47
Copy link
Copy Markdown
Contributor

@dzhidzhoev dzhidzhoev Jun 1, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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:

  1. If dxc invocation contains /Fd=something, PDB file named "something" is created.
  2. 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?

Copy link
Copy Markdown
Contributor

@asavonic asavonic Jun 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Author

@kuilpd kuilpd Jun 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

@dzhidzhoev dzhidzhoev Jun 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

static cl::opt<bool> ShaderHashDependsOnSource(
"dx-Zss", cl::desc("Compute Shader Hash considering source information"));
extern cl::opt<bool> SourceInDebugModule;
Expand Down Expand Up @@ -149,9 +146,9 @@ void DXContainerGlobals::computeShaderHashAndDebugName(
}

Digest.update(DXILConstant->getRawDataValues());
MD5::MD5Result Result = Digest.final();
MD5::MD5Result MD5 = Digest.final();

memcpy(reinterpret_cast<void *>(&HashData.Digest), Result.data(), 16);
memcpy(reinterpret_cast<void *>(&HashData.Digest), MD5.data(), 16);
if (sys::IsBigEndianHost)
HashData.swapBytes();
StringRef Data(reinterpret_cast<char *>(&HashData), sizeof(dxbc::ShaderHash));
Expand All @@ -168,38 +165,40 @@ void DXContainerGlobals::computeShaderHashAndDebugName(
if (!MMI.SourceInfo)
return;

if (!PdbFileName.empty() && !PdbOutputDir.empty())
report_fatal_error(
"--dx-pdb-file and --dx-pdb-dir are mutually exclusive options");
if (!EmbedDebug && PdbDebugPath.empty())
return;
Comment on lines +168 to +169
Copy link
Copy Markdown
Contributor

@dzhidzhoev dzhidzhoev May 31, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Author

@kuilpd kuilpd Jun 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

@dzhidzhoev dzhidzhoev Jun 1, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


SmallString<40> DebugNameStr;
mcdxbc::DebugName DebugName;
if (PdbFileName.empty()) {
// TODO Add an option to compute hash based on ILDB.
Digest.stringifyResult(Result, DebugNameStr);
DebugNameStr += ".pdb";
} else {
// Use user-provided PDB file name.
DebugNameStr = PdbFileName;
}
DebugName.setFilename(DebugNameStr);
Digest.stringifyResult(MD5, DebugNameStr);
DebugNameStr += ".pdb";
if (!PdbDebugPath.empty()) {
StringRef DebugFile = PdbDebugPath.getValue();
SmallString<256> AbsoluteDebugName;
if (sys::path::is_separator(DebugFile.back())) {
// If /Fd was specified as a directory, put the MD5.pdb file there.
AbsoluteDebugName = DebugFile;
sys::path::append(AbsoluteDebugName, DebugNameStr);
} else {
// Otherwise, use /Fd value as a user-provided PDB file name.
DebugNameStr = DebugFile;
AbsoluteDebugName = DebugNameStr;
}

SmallString<256> AbsoluteDebugName(PdbOutputDir);
sys::path::append(AbsoluteDebugName, DebugNameStr);
// Pass PDB name to DXContainerPDBPass via PDBNAME section.
addSection(M, Globals, AbsoluteDebugName, "dx.pdb.name",
PdbFileNameSectionName);
// Pass module hash to DXContainerPDBPass.
Globals.emplace_back(buildContainerGlobal(
M, ConstantDataArray::get(M.getContext(), ArrayRef(HashData.Digest)),
"dx.pdb.hash", ModuleHashSectionName));
}

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

// TODO Do not create PDB in embedded mode.
// Pass PDB name to DXContainerPDBPass via PDBNAME section.
addSection(M, Globals, AbsoluteDebugName, "dx.pdb.name",
PdbFileNameSectionName);
// Pass module hash to DXContainerPDBPass.
Globals.emplace_back(buildContainerGlobal(
M, ConstantDataArray::get(M.getContext(), ArrayRef(HashData.Digest)),
"dx.pdb.hash", ModuleHashSectionName));
}

GlobalVariable *DXContainerGlobals::buildContainerGlobal(
Expand Down
Loading