Always write shader hash when supported, don't tie to debug name (#2468)

* Always write shader hash when supported, don't tie to debug name

* Stop trimming quotes from pdb name

This is the responsibility of cmd.exe, not the API.
The API user should not be adding extra quotes to the pdb name string
if they don't want them.

lib/DxilContainer/DxilContainerAssembler.cpp

@@ -1752,73 +1752,72 @@ void hlsl::SerializeDxilContainerForModule(DxilModule *pModule,
     WriteBitcodeToFile(pModule->GetModule(), outStream, false);
   }
 
-  // Serialize debug name if requested.
-  CComPtr<AbstractMemoryStream> pHashStream;
-  std::string DebugNameStr; // Used if constructing name based on hash
+  // Compute hash if needed.
   DxilShaderHash HashContent;
-  if (pShaderHashOut || Flags & SerializeDxilFlags::IncludeDebugNamePart) {
+  SmallString<32> HashStr;
+  if (bSupportsShaderHash || pShaderHashOut ||
+      (Flags & SerializeDxilFlags::IncludeDebugNamePart &&
+        DebugName.empty()))
+  {
     // If the debug name should be specific to the sources, base the name on the debug
     // bitcode, which will include the source references, line numbers, etc. Otherwise,
     // do it exclusively on the target shader bitcode.
+    llvm::MD5 md5;
     if (Flags & SerializeDxilFlags::DebugNameDependOnSource) {
-      pHashStream = CComPtr<AbstractMemoryStream>(pModuleBitcode);
+      md5.update(ArrayRef<uint8_t>(pModuleBitcode->GetPtr(), pModuleBitcode->GetPtrSize()));
       HashContent.Flags = (uint32_t)DxilShaderHashFlags::IncludesSource;
     } else {
-      pHashStream = CComPtr<AbstractMemoryStream>(pProgramStream);
+      md5.update(ArrayRef<uint8_t>(pProgramStream->GetPtr(), pProgramStream->GetPtrSize()));
       HashContent.Flags = (uint32_t)DxilShaderHashFlags::None;
     }
-
-    ArrayRef<uint8_t> Data((uint8_t *)pHashStream->GetPtr(),
-                           pHashStream->GetPtrSize());
-    llvm::MD5 md5;
-    SmallString<32> Hash;
-    md5.update(Data);
     md5.final(HashContent.Digest);
+    md5.stringifyResult(HashContent.Digest, HashStr);
+  }
 
-    if (Flags & SerializeDxilFlags::IncludeDebugNamePart) {
-      if (DebugName.empty()) {
-        md5.stringifyResult(HashContent.Digest, Hash);
-        DebugNameStr += Hash;
-        DebugNameStr += ".pdb";
-        DebugName = DebugNameStr;
-      }
-
-      DebugName = DebugName.trim("\"");
-
-      // Calculate the size of the blob part.
-      const uint32_t DebugInfoContentLen = PSVALIGN4(
-          sizeof(DxilShaderDebugName) + DebugName.size() + 1); // 1 for null
-
-      writer.AddPart(DFCC_ShaderDebugName, DebugInfoContentLen,
-        [DebugName]
-        (AbstractMemoryStream *pStream)
-      {
-        DxilShaderDebugName NameContent;
-        NameContent.Flags = 0;
-        NameContent.NameLength = DebugName.size();
-        IFT(WriteStreamValue(pStream, NameContent));
-
-        ULONG cbWritten;
-        IFT(pStream->Write(DebugName.begin(), DebugName.size(), &cbWritten));
-        const char Pad[] = { '\0','\0','\0','\0' };
-        // Always writes at least one null to align size
-        unsigned padLen = (4 - ((sizeof(DxilShaderDebugName) + cbWritten) & 0x3));
-        IFT(pStream->Write(Pad, padLen, &cbWritten));
-      });
-    }
-
-    if (bSupportsShaderHash) {
-      writer.AddPart(DFCC_ShaderHash, sizeof(HashContent),
-        [HashContent]
-        (AbstractMemoryStream *pStream)
-      {
-        IFT(WriteStreamValue(pStream, HashContent));
-      });
-    }
+  // Serialize debug name if requested.
+  std::string DebugNameStr; // Used if constructing name based on hash
+  if (Flags & SerializeDxilFlags::IncludeDebugNamePart) {
+    if (DebugName.empty()) {
+      DebugNameStr += HashStr;
+      DebugNameStr += ".pdb";
+      DebugName = DebugNameStr;
+    }
+
+    // Calculate the size of the blob part.
+    const uint32_t DebugInfoContentLen = PSVALIGN4(
+        sizeof(DxilShaderDebugName) + DebugName.size() + 1); // 1 for null
+
+    writer.AddPart(DFCC_ShaderDebugName, DebugInfoContentLen,
+      [DebugName]
+      (AbstractMemoryStream *pStream)
+    {
+      DxilShaderDebugName NameContent;
+      NameContent.Flags = 0;
+      NameContent.NameLength = DebugName.size();
+      IFT(WriteStreamValue(pStream, NameContent));
 
-    if (pShaderHashOut) {
-      memcpy(pShaderHashOut, &HashContent, sizeof(DxilShaderHash));
-    }
+      ULONG cbWritten;
+      IFT(pStream->Write(DebugName.begin(), DebugName.size(), &cbWritten));
+      const char Pad[] = { '\0','\0','\0','\0' };
+      // Always writes at least one null to align size
+      unsigned padLen = (4 - ((sizeof(DxilShaderDebugName) + cbWritten) & 0x3));
+      IFT(pStream->Write(Pad, padLen, &cbWritten));
+    });
+  }
+
+  // Add hash to container if supported by validator version.
+  if (bSupportsShaderHash) {
+    writer.AddPart(DFCC_ShaderHash, sizeof(HashContent),
+      [HashContent]
+      (AbstractMemoryStream *pStream)
+    {
+      IFT(WriteStreamValue(pStream, HashContent));
+    });
+  }
+
+  // Write hash to separate output if requested.
+  if (pShaderHashOut) {
+    memcpy(pShaderHashOut, &HashContent, sizeof(DxilShaderHash));
   }
 
   // Compute padded bitcode size.

tools/clang/tools/dxcompiler/dxcompilerobj.cpp

@@ -756,12 +756,12 @@ public:
             valHR = dxcutil::ValidateAndAssembleToContainer(
                 action.takeModule(), pOutputBlob, m_pMalloc, SerializeFlags,
                 pOutputStream, opts.IsDebugInfoEnabled(), opts.GetPDBName(), compiler.getDiagnostics(),
-                (SerializeFlags & SerializeDxilFlags::IncludeDebugNamePart) ? &ShaderHashContent : nullptr);
+                &ShaderHashContent);
           } else {
             dxcutil::AssembleToContainer(action.takeModule(),
                                          pOutputBlob, m_pMalloc,
                                          SerializeFlags, pOutputStream,
-                (SerializeFlags & SerializeDxilFlags::IncludeDebugNamePart) ? &ShaderHashContent : nullptr);
+                                         &ShaderHashContent);
           }
 
           // Callback after valid DXIL is produced

tools/clang/unittests/HLSL/CompilerTest.cpp

@@ -288,7 +288,6 @@ public:
   TEST_METHOD(BatchValidation)
 
   TEST_METHOD(SubobjectCodeGenErrors)
-  TEST_METHOD(SanitizePDBName)
   BEGIN_TEST_METHOD(ManualFileCheckTest)
     TEST_METHOD_PROPERTY(L"Ignore", L"true")
   END_TEST_METHOD()
@@ -2785,55 +2784,6 @@ TEST_F(CompilerTest, SubobjectCodeGenErrors) {
   }
 }
 
-// Check that pdb name doesn't contain any preceding or trailing quotations
-TEST_F(CompilerTest, SanitizePDBName) {
-  const char *hlsl = R"(
-    [RootSignature("")]
-    float main(float pos : A) : SV_Target {
-      float x = abs(pos);
-      float y = sin(pos);
-      float z = x + y;
-      return z;
-    }
-  )";
-  CComPtr<IDxcLibrary> pLib;
-  VERIFY_SUCCEEDED(m_dllSupport.CreateInstance(CLSID_DxcLibrary, &pLib));
-
-  CComPtr<IDxcCompiler> pCompiler;
-  CComPtr<IDxcCompiler2> pCompiler2;
-
-  CComPtr<IDxcOperationResult> pResult;
-  CComPtr<IDxcBlobEncoding> pSource;
-  CComPtr<IDxcBlob> pProgram;
-  CComPtr<IDxcBlob> pPdbBlob;
-  WCHAR *pDebugName = nullptr;
-
-  VERIFY_SUCCEEDED(CreateCompiler(&pCompiler));
-  VERIFY_SUCCEEDED(pCompiler.QueryInterface(&pCompiler2));
-  CreateBlobFromText(hlsl, &pSource);
-  LPCWSTR args[] = { L"/Zi", L"/Fd",
-    L"\"my_pdb.pdb\"" // With Added Quotations
-  };
-  VERIFY_SUCCEEDED(pCompiler2->CompileWithDebug(pSource, L"source.hlsl", L"main",
-    L"ps_6_0", args, _countof(args), nullptr, 0, nullptr, &pResult, &pDebugName, &pPdbBlob));
-  VERIFY_SUCCEEDED(pResult->GetResult(&pProgram));
-
-  VERIFY_IS_TRUE(pDebugName && 0 == wcscmp(pDebugName, L"my_pdb.pdb"));
-
-  CComPtr<IDxcContainerReflection> pReflection;
-  VERIFY_SUCCEEDED(m_dllSupport.CreateInstance(CLSID_DxcContainerReflection, &pReflection));
-  VERIFY_SUCCEEDED(pReflection->Load(pProgram));
-
-  CComPtr<IDxcBlob> pNameBlob;
-  UINT32 uDebugNameIndex = 0;
-  VERIFY_SUCCEEDED(pReflection->FindFirstPartKind(hlsl::DFCC_ShaderDebugName, &uDebugNameIndex));
-  VERIFY_SUCCEEDED(pReflection->GetPartContent(uDebugNameIndex, &pNameBlob));
-
-  auto pName = (hlsl::DxilShaderDebugName *)pNameBlob->GetBufferPointer();
-  const char *Name = (char *)&pName[1];
-  VERIFY_IS_TRUE(0 == strcmp(Name, "my_pdb.pdb"));
-}
-
 #ifdef _WIN32
 TEST_F(CompilerTest, ManualFileCheckTest) {
 #else