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

Author: kuilpd

clang/include/clang/Basic/DiagnosticDriverKinds.td

@@ -883,6 +883,16 @@ def err_drv_dxc_Fre_requires_Fo_metal
     : Error<"-Fre option requires -Fo option when targeting Metal">;
 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<

clang/include/clang/Options/Options.td

@@ -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>;
@@ -9662,6 +9661,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">,
@@ -9765,11 +9776,9 @@ def spirv_validator_path_EQ : Joined<["--"], "spirv-val-path=">, Group<dxc_Group
   HelpText<"SPIRV 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">;

clang/lib/Driver/ToolChains/Clang.cpp

@@ -3851,6 +3851,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,

clang/test/Driver/dxc_debug.hlsl

@@ -1,8 +1,10 @@
 // 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 /Qembed_debug %s 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK-EMBED
 // 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 -Qembed_debug %s 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK-EMBED
+// RUN: %clang_dxc -Tlib_6_7 -### -Zi /Fd %t.pdb %s 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK-FD
+// RUN: %clang_dxc -Tlib_6_7 -### -Zi -Fd=%t.pdb %s 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK-FD
 // 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
@@ -13,6 +15,17 @@
 // CHECK-SAME: "-debug-info-kind=constructor"
 // Make sure dwarf-version is 4.
 // CHECK-DWARF-SAME: -dwarf-version=4
+// Check that -Qembed_debug and -Fd flags are converted to llc equivalents
+// CHECK-EMBED-SAME: --dx-embed-debug
+// CHECK-FD-SAME: --dx-Fd=
 // 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
+
+// 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

llvm/include/llvm/MC/MCDXContainerWriter.h

@@ -73,6 +73,7 @@ class LLVM_ABI DXContainerObjectWriter final : public MCDXContainerBaseWriter,
 
 protected:
   ArrayRef<MCDXContainerPart> getParts() override;
+  bool shouldSkipSection(StringRef SectionName, size_t SectionSize) override;
 
 public:
   DXContainerObjectWriter(std::unique_ptr<MCDXContainerTargetWriter> MOTW,

llvm/lib/MC/MCDXContainerWriter.cpp

@@ -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;
@@ -132,6 +136,16 @@ 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;
+  if (SectionName == "SRCI")
+    return true;
+  return MCDXContainerBaseWriter::shouldSkipSection(SectionName, SectionSize);
+}
+
 uint64_t DXContainerObjectWriter::writeObject() {
   write(W.OS, getContext().getTargetTriple());
   clearParts();

llvm/lib/Target/DirectX/DXContainerGlobals.cpp

@@ -39,15 +39,11 @@ 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"));
+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"));
 static cl::opt<bool> ShaderHashDependsOnSource(
     "dx-Zss", cl::desc("Compute Shader Hash considering source information"));
 extern cl::opt<bool> SourceInDebugModule;
@@ -151,9 +147,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));
@@ -163,47 +159,41 @@ void DXContainerGlobals::computeShaderHashAndDebugName(
   Globals.emplace_back(
       buildContainerGlobal(M, ModuleConstant, "dx.hash", "HASH"));
 
-  // Emit ILDN part in debug info mode.
-  // DXIL bitcode hash is used, which corresponds to DXC behavior with
-  // `/Zi /Qembed_debug /Zsb` flags.
   if (M.debug_compile_units().empty())
     return;
 
-  if (!PdbFileName.empty() && !PdbOutputDir.empty())
-    report_fatal_error(
-        "--dx-pdb-file and --dx-pdb-dir are mutually exclusive options");
-
   SmallString<40> DebugNameStr;
-  Digest.stringifyResult(Result, 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;
+    }
 
-  mcdxbc::DebugName DebugName;
-  if (PdbFileName.empty()) {
-    // Use the MD5 hash as the file name.
-    Digest.stringifyResult(Result, DebugNameStr);
-    DebugNameStr += ".pdb";
-  } else {
-    // Use user-provided PDB file name.
-    DebugNameStr = PdbFileName;
+    // 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));
   }
-  DebugName.setFilename(DebugNameStr);
-
-  SmallString<256> AbsoluteDebugName(PdbOutputDir);
-  sys::path::append(AbsoluteDebugName, DebugNameStr);
 
+  // Emit ILDN part in debug info mode.
+  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(

llvm/lib/Target/DirectX/DXILWriter/DXILWriterPass.cpp

@@ -36,6 +36,8 @@
 using namespace llvm;
 using namespace llvm::dxil;
 
+extern cl::opt<bool> EmbedDebug;
+extern cl::opt<std::string> PdbDebugPath;
 // NOLINTNEXTLINE(misc-use-internal-linkage)
 cl::opt<bool> SourceInDebugModule(
     "dx-source-in-debug-module",
@@ -236,6 +238,18 @@ class EmbedDXILPass : public llvm::ModulePass {
     legalizeLifetimeIntrinsics(M);
 
     bool HasDebugInfo = !M.debug_compile_units().empty();
+
+    // Enable EmbedDebug if there is debug info, but it is not being written
+    // to a PDB file.
+    if (HasDebugInfo && !EmbedDebug && PdbDebugPath.empty())
+      EmbedDebug = true;
+    if (!HasDebugInfo && EmbedDebug)
+      reportFatalUsageError(
+          "Missing debug info for embedding into the container");
+    // TODO: move this check to DXContainerPDB.cpp when /Zs is implemented.
+    if (!HasDebugInfo && !PdbDebugPath.empty())
+      reportFatalUsageError("Missing debug info for writing to the PDB file");
+
     std::string ILDBData;
     if (HasDebugInfo) {
       // Write DXIL with debug info to ILDB part.

llvm/test/CodeGen/DirectX/ContainerData/ContainerFlags.ll

@@ -0,0 +1,38 @@
+;; Check that --dx-embed-debug is enabled by default when debug info is present
+; RUN: llc %S/Inputs/SourceInfo.ll --filetype=obj -o %t.cso
+; RUN: obj2yaml %t.cso | FileCheck %s --check-prefix=DEFAULT-EMBED
+; DEFAULT-EMBED: Parts:
+; DEFAULT-EMBED:   - Name:            ILDB
+; DEFAULT-EMBED:   - Name:            ILDN
+
+;; Check that debug info is not embedded if only the PDB ouput is specified
+; RUN: llc %S/Inputs/SourceInfo.ll --filetype=obj --dx-Fd=%t.pdb -o %t.cso
+; RUN: obj2yaml %t.cso | FileCheck %s --check-prefix=NO-EMBED
+; NO-EMBED: Parts:
+; NO-EMBED-NOT:   - Name:            ILDB
+; NO-EMBED:       - Name:            ILDN
+
+;; Check that debug info is both embedded and output to the PDB
+;; if both options are specified
+; RUN: llc %S/Inputs/SourceInfo.ll --filetype=obj --dx-embed-debug --dx-Fd=%t.pdb -o %t.cso
+; RUN: obj2yaml %t.cso | FileCheck %s --check-prefix=EMBED
+; EMBED: Parts:
+; EMBED:   - Name:            ILDB
+; EMBED:   - Name:            ILDN
+; RUN: llvm-pdbutil dump --dxcontainer %t.pdb | FileCheck %s --check-prefix=PDB
+; Check that PDB file contains only debug-info relevant parts.
+; PDB: Parts:
+; PDB:   ILDB
+; PDB:   ILDN
+
+;; Check errors when trying to output debug info with no debug info present
+; RUN: not llc %s --filetype=obj --dx-embed-debug -o %t.cso 2>&1 | FileCheck %s --check-prefix=ERROR-NODBG
+; ERROR-NODBG: Missing debug info for embedding into the container
+; RUN: not llc %s --filetype=obj --dx-Fd=%t.pdb -o %t.cso 2>&1 | FileCheck %s --check-prefix=ERROR-NODBG-PDB
+; ERROR-NODBG-PDB: Missing debug info for writing to the PDB file
+
+target triple = "dxil-unknown-shadermodel6.5-library"
+
+define i32 @foo(i32 %a) {
+  ret i32 %a
+}

llvm/test/CodeGen/DirectX/ContainerData/DebugName-default-output.test

@@ -1,9 +1,8 @@
-RUN: llc %S/Inputs/SourceInfo.ll -dx-Zss --filetype=obj -o %t.dxbc
+RUN: llc %S/Inputs/SourceInfo.ll -dx-Zss --dx-embed-debug --filetype=obj -o %t.dxbc
 RUN: llvm-objcopy --dump-section=ILDB=%t0.bc %t.dxbc
 RUN: %md5sum %t0.bc >%t0.bc.md5
 RUN: obj2yaml %t.dxbc >%t.yaml
 RUN: cat %t.yaml %t0.bc.md5 | FileCheck %s
-RUN: %python %S/Inputs/check_pdb_exists.py "" "%t0.bc.md5"
 
 CHECK-NOT:   - Name:            PDBN
 CHECK-NOT:   - Name:            PDBH