Browse Source

Fix issues with -Qstrip_reflect and -Qstrip_debug (#2164)

-Qstrip_reflect would reserialize the root signature, leading to
validation failure #2162.  Fixed by moving root sig to writer to clear
from module and prevent re-serialization to metadata.

Fixed -Qstrip_debug with -Zi and no output location still embeding
debug module.
Tex Riddell 6 years ago
parent
commit
638d988e8f

+ 1 - 0
include/dxc/DXIL/DxilModule.h

@@ -121,6 +121,7 @@ public:
   DxilSignature &GetPatchConstantSignature();
   const DxilSignature &GetPatchConstantSignature() const;
   const std::vector<uint8_t> &GetSerializedRootSignature() const;
+  std::vector<uint8_t> &GetSerializedRootSignature();
 
   bool HasDxilEntrySignature(const llvm::Function *F) const;
   DxilEntrySignature &GetDxilEntrySignature(const llvm::Function *F);

+ 4 - 0
lib/DXIL/DxilModule.cpp

@@ -945,6 +945,10 @@ const std::vector<uint8_t> &DxilModule::GetSerializedRootSignature() const {
   return m_SerializedRootSignature;
 }
 
+std::vector<uint8_t> &DxilModule::GetSerializedRootSignature() {
+  return m_SerializedRootSignature;
+}
+
 // Entry props.
 bool DxilModule::HasDxilEntrySignature(const llvm::Function *F) const {
   return m_DxilEntryPropsMap.find(F) != m_DxilEntryPropsMap.end();

+ 5 - 4
lib/DxilContainer/DxilContainerAssembler.cpp

@@ -1448,10 +1448,10 @@ namespace {
 
 class RootSignatureWriter : public DxilPartWriter {
 private:
-  const std::vector<uint8_t> &m_Sig;
+  std::vector<uint8_t> m_Sig;
 
 public:
-  RootSignatureWriter(const std::vector<uint8_t> &S) : m_Sig(S) {}
+  RootSignatureWriter(std::vector<uint8_t> &&S) : m_Sig(std::move(S)) {}
   uint32_t size() const { return m_Sig.size(); }
   void write(AbstractMemoryStream *pStream) {
     ULONG cbWritten;
@@ -1527,7 +1527,8 @@ void hlsl::SerializeDxilContainerForModule(DxilModule *pModule,
   std::unique_ptr<DxilPSVWriter> pPSVWriter = nullptr;
   unsigned int major, minor;
   pModule->GetDxilVersion(major, minor);
-  RootSignatureWriter rootSigWriter(pModule->GetSerializedRootSignature());
+  RootSignatureWriter rootSigWriter(std::move(pModule->GetSerializedRootSignature())); // Grab RS here
+  DXASSERT_NOMSG(pModule->GetSerializedRootSignature().empty());
 
   bool bMetadataStripped = false;
   if (pModule->GetShaderModel()->IsLib()) {
@@ -1546,7 +1547,7 @@ void hlsl::SerializeDxilContainerForModule(DxilModule *pModule,
         DFCC_PipelineStateValidation, pPSVWriter->size(),
         [&](AbstractMemoryStream *pStream) { pPSVWriter->write(pStream); });
     // Write the root signature (RTS0) part.
-    if (!pModule->GetSerializedRootSignature().empty()) {
+    if (rootSigWriter.size()) {
       writer.AddPart(
         DFCC_RootSignature, rootSigWriter.size(),
         [&](AbstractMemoryStream *pStream) { rootSigWriter.write(pStream); });

+ 1 - 1
tools/clang/tools/dxcompiler/dxcompilerobj.cpp

@@ -581,7 +581,7 @@ public:
         // embed the debug info and emit a note.
         if (opts.EmbedDebugInfo()) {
           SerializeFlags |= SerializeDxilFlags::IncludeDebugInfoPart;
-        } else if (opts.DebugInfo && !ppDebugBlob) {
+        } else if (!opts.StripDebug && opts.DebugInfo && !ppDebugBlob) {
           w << "warning: no output provided for debug - embedding PDB in shader container.  Use -Qembed_debug to silence this warning.\n";
           SerializeFlags |= SerializeDxilFlags::IncludeDebugInfoPart;
         }

+ 35 - 0
utils/hct/hcttestcmds.cmd

@@ -155,6 +155,41 @@ if %errorlevel% neq 0 (
   exit /b 1
 )
 
+rem /Zi with /Qstrip_debug and no output should not embed
+dxc.exe /T ps_6_0 "%testfiles%\smoke.hlsl" /Zi /Qstrip_debug /Fo smoke.hlsl.strip 1>nul
+if %errorlevel% neq 0 (
+  echo Failed - %CD%\dxc.exe /T ps_6_0 "%testfiles%\smoke.hlsl" /Fd %CD%\
+  call :cleanup 2>nul
+  exit /b 1
+)
+rem auto debug name is hex digest + .lld
+dxc.exe -dumpbin smoke.hlsl.strip | findstr -r -c:"shader debug name: [0-9a-f]*.lld" 1>nul
+if %errorlevel% neq 0 (
+  echo Failed to find shader debug name.
+  call :cleanup 2>nul
+  exit /b 1
+)
+dxc.exe -dumpbin smoke.hlsl.strip | findstr "DICompileUnit" 1>nul
+if %errorlevel% equ 0 (
+  echo Found DICompileUnit when -Qstrip_debug used.
+  call :cleanup 2>nul
+  exit /b 1
+)
+
+rem /Qstrip_reflect strips reflection
+dxc.exe /T ps_6_0 "%testfiles%\smoke.hlsl" -D DX12 /Qstrip_reflect /Fo smoke.hlsl.strip 1>nul
+if %errorlevel% neq 0 (
+  echo Failed - %CD%\dxc.exe /T ps_6_0 "%testfiles%\smoke.hlsl" /Fd %CD%\
+  call :cleanup 2>nul
+  exit /b 1
+)
+dxc.exe -dumpbin smoke.hlsl.strip | findstr -c:"i32 6, !\"g\"" 1>nul
+if %errorlevel% equ 0 (
+  echo Found reflection when -Qstrip_reflect used.
+  call :cleanup 2>nul
+  exit /b 1
+)
+
 dxc.exe /T ps_6_0 "%testfiles%\smoke.hlsl" /Fe smoke.hlsl.e 1>nul
 if %errorlevel% neq 0 (
   echo Failed - %CD%\dxc.exe /T ps_6_0 "%testfiles%\smoke.hlsl" /Fe %CD%\smoke.hlsl.e