Dedup semantic names in IO signature parts for valver 1.7 (#4406)

Create new NameOffsetMap that uses StringRef to prevent identical strings from being duplicated. Old behavior is maintained when earlier validator version set by using a *_nodedup map that continues to use a pointer to allow duplicates as previous validators did.

include/dxc/Test/DxcTestUtils.h

@@ -119,6 +119,7 @@ private:
   FileRunCommandResult RunDxc(dxc::DxcDllSupport &DllSupport, const FileRunCommandResult *Prior);
   FileRunCommandResult RunDxv(dxc::DxcDllSupport &DllSupport, const FileRunCommandResult *Prior);
   FileRunCommandResult RunOpt(dxc::DxcDllSupport &DllSupport, const FileRunCommandResult *Prior);
+  FileRunCommandResult RunListParts(dxc::DxcDllSupport &DllSupport, const FileRunCommandResult *Prior);
   FileRunCommandResult RunD3DReflect(dxc::DxcDllSupport &DllSupport, const FileRunCommandResult *Prior);
   FileRunCommandResult RunDxr(dxc::DxcDllSupport &DllSupport, const FileRunCommandResult *Prior);
   FileRunCommandResult RunLink(dxc::DxcDllSupport &DllSupport, const FileRunCommandResult *Prior);

lib/DxilContainer/DxilContainerAssembler.cpp

@@ -179,11 +179,14 @@ private:
   bool   m_isInput;
   bool   m_useMinPrecision;
   bool m_bCompat_1_4;
-  bool m_bUnaligned; // unaligned size for < 1.7
+  bool m_bCompat_1_6; // unaligned size, no dedup for < 1.7
   size_t m_fixedSize;
-  typedef std::pair<const char *, uint32_t> NameOffsetPair;
-  typedef llvm::SmallMapVector<const char *, uint32_t, 8> NameOffsetMap;
+  typedef std::pair<const char *, uint32_t> NameOffsetPair_nodedup;
+  typedef llvm::SmallMapVector<const char *, uint32_t, 8> NameOffsetMap_nodedup;
+  typedef std::pair<llvm::StringRef, uint32_t> NameOffsetPair;
+  typedef llvm::SmallMapVector<llvm::StringRef, uint32_t, 8> NameOffsetMap;
   uint32_t m_lastOffset;
+  NameOffsetMap_nodedup m_semanticNameOffsets_nodedup;
   NameOffsetMap m_semanticNameOffsets;
   unsigned m_paramCount;
 
@@ -193,13 +196,13 @@ private:
     return pElement->GetName();
   }
 
-  uint32_t GetSemanticOffset(const hlsl::DxilSignatureElement *pElement) {
+  uint32_t GetSemanticOffset_nodedup(const hlsl::DxilSignatureElement *pElement) {
     const char *pName = GetSemanticName(pElement);
-    NameOffsetMap::iterator nameOffset = m_semanticNameOffsets.find(pName);
+    NameOffsetMap_nodedup::iterator nameOffset = m_semanticNameOffsets_nodedup.find(pName);
     uint32_t result;
-    if (nameOffset == m_semanticNameOffsets.end()) {
+    if (nameOffset == m_semanticNameOffsets_nodedup.end()) {
       result = m_lastOffset;
-      m_semanticNameOffsets.insert(NameOffsetPair(pName, result));
+      m_semanticNameOffsets_nodedup.insert(NameOffsetPair_nodedup(pName, result));
       m_lastOffset += strlen(pName) + 1;
     }
     else {
@@ -207,6 +210,23 @@ private:
     }
     return result;
   }
+  uint32_t GetSemanticOffset(const hlsl::DxilSignatureElement *pElement) {
+    if (m_bCompat_1_6)
+      return GetSemanticOffset_nodedup(pElement);
+
+    StringRef name = GetSemanticName(pElement);
+    NameOffsetMap::iterator nameOffset = m_semanticNameOffsets.find(name);
+    uint32_t result;
+    if (nameOffset == m_semanticNameOffsets.end()) {
+      result = m_lastOffset;
+      m_semanticNameOffsets.insert(NameOffsetPair(name, result));
+      m_lastOffset += name.size() + 1;
+    }
+    else {
+      result = nameOffset->second;
+    }
+    return result;
+  }
 
   void write(std::vector<DxilProgramSignatureElement> &orderedSig,
              const hlsl::DxilSignatureElement *pElement) {
@@ -285,16 +305,16 @@ public:
                              DXIL::TessellatorDomain domain,
                              bool isInput, bool UseMinPrecision,
                              bool bCompat_1_4,
-                             bool bUnaligned)
+                             bool bCompat_1_6)
       : m_signature(signature), m_domain(domain),
         m_isInput(isInput), m_useMinPrecision(UseMinPrecision),
         m_bCompat_1_4(bCompat_1_4),
-        m_bUnaligned(bUnaligned) {
+        m_bCompat_1_6(bCompat_1_6) {
     calcSizes();
   }
 
   uint32_t size() const override {
-    if (m_bUnaligned)
+    if (m_bCompat_1_6)
       return m_lastOffset;
     else
       return PSVALIGN4(m_lastOffset);
@@ -325,19 +345,23 @@ public:
 
     // Write strings in the offset order.
     std::vector<NameOffsetPair> ordered;
-    ordered.assign(m_semanticNameOffsets.begin(), m_semanticNameOffsets.end());
+    if (m_bCompat_1_6) {
+      ordered.assign(m_semanticNameOffsets_nodedup.begin(), m_semanticNameOffsets_nodedup.end());
+    } else {
+      ordered.assign(m_semanticNameOffsets.begin(), m_semanticNameOffsets.end());
+    }
     std::sort(ordered.begin(), ordered.end(), sort_second<NameOffsetPair>());
     for (size_t i = 0; i < ordered.size(); ++i) {
-      const char *pName = ordered[i].first;
+      StringRef name = ordered[i].first;
       ULONG cbWritten;
       UINT64 offsetPos = pStream->GetPosition();
       DXASSERT_LOCALVAR(offsetPos, offsetPos - startPos == ordered[i].second, "else str offset is incorrect");
-      IFT(pStream->Write(pName, strlen(pName) + 1, &cbWritten));
+      IFT(pStream->Write(name.data(), name.size() + 1, &cbWritten));
     }
 
     // Align, and verify we wrote the same number of bytes we though we would.
     UINT64 bytesWritten = pStream->GetPosition() - startPos;
-    if (!m_bUnaligned && (bytesWritten % 4 != 0)) {
+    if (!m_bCompat_1_6 && (bytesWritten % 4 != 0)) {
       unsigned paddingToAdd = 4 - (bytesWritten % 4);
       char padding[4] = {0};
       ULONG cbWritten = 0;
@@ -355,24 +379,24 @@ DxilPartWriter *hlsl::NewProgramSignatureWriter(const DxilModule &M, DXIL::Signa
   unsigned ValMajor, ValMinor;
   M.GetValidatorVersion(ValMajor, ValMinor);
   bool bCompat_1_4 = DXIL::CompareVersions(ValMajor, ValMinor, 1, 5) < 0;
-  bool bUnaligned = DXIL::CompareVersions(ValMajor, ValMinor, 1, 7) < 0;
+  bool bCompat_1_6 = DXIL::CompareVersions(ValMajor, ValMinor, 1, 7) < 0;
   switch (Kind) {
   case DXIL::SignatureKind::Input:
     return new DxilProgramSignatureWriter(
         M.GetInputSignature(), domain, true,
         M.GetUseMinPrecision(),
-        bCompat_1_4, bUnaligned);
+        bCompat_1_4, bCompat_1_6);
   case DXIL::SignatureKind::Output:
     return new DxilProgramSignatureWriter(
         M.GetOutputSignature(), domain, false,
         M.GetUseMinPrecision(),
-        bCompat_1_4, bUnaligned);
+        bCompat_1_4, bCompat_1_6);
   case DXIL::SignatureKind::PatchConstOrPrim:
     return new DxilProgramSignatureWriter(
         M.GetPatchConstOrPrimSignature(), domain,
         /*IsInput*/ M.GetShaderModel()->IsDS(),
         /*UseMinPrecision*/M.GetUseMinPrecision(),
-        bCompat_1_4, bUnaligned);
+        bCompat_1_4, bCompat_1_6);
   case DXIL::SignatureKind::Invalid:
     return nullptr;
   }

tools/clang/test/HLSLFileCheck/hlsl/signature/sig-dedup.hlsl

@@ -0,0 +1,10 @@
+// RUN: %dxilver 1.4 | %dxc -E main -T ps_6_0 -validator-version 1.4 %s | %listparts | FileCheck %s -check-prefix=NODEDUP
+// RUN: %dxilver 1.7 | %dxc -E main -T ps_6_0 -validator-version 1.7 %s | %listparts | FileCheck %s -check-prefix=DEDUP
+
+// NODEDUP: #1 - ISG1 (124 bytes)
+// DEDUP: #1 - ISG1 (116 bytes)
+
+float4 main(float4 f0 : TEXCOORD0, uint2 u1 : U1, float2 f2 : TEXCOORD2) : SV_Target
+{
+	return f0 + (u1 * f2).xyxy;
+}

tools/clang/unittests/HLSLTestLib/FileCheckerTest.cpp

@@ -107,6 +107,15 @@ FileRunCommandResult FileRunCommandPart::Run(dxc::DxcDllSupport &DllSupport, con
   else if (0 == _stricmp(Command.c_str(), "%opt")) {
     return RunOpt(DllSupport, Prior);
   }
+  else if (0 == _stricmp(Command.c_str(), "%listparts")) {
+#ifdef _WIN32 // Reflection unsupported
+    return RunListParts(DllSupport, Prior);
+#else
+    FileRunCommandResult result = FileRunCommandResult::Success("Can't run listparts on non-windows, so just assuming success");
+    result.AbortPipeline = true;
+    return result;
+#endif // WIN32 - Reflection unsupported
+  }
   else if (0 == _stricmp(Command.c_str(), "%D3DReflect")) {
 #ifdef _WIN32 // Reflection unsupported
     return RunD3DReflect(DllSupport, Prior);
@@ -658,6 +667,71 @@ FileRunCommandResult FileRunCommandPart::RunOpt(dxc::DxcDllSupport &DllSupport,
 }
 
 #ifdef _WIN32 // Reflection unsupported
+FileRunCommandResult FileRunCommandPart::RunListParts(dxc::DxcDllSupport &DllSupport, const FileRunCommandResult *Prior) {
+  std::string args(strtrim(Arguments));
+  const char *inputPos = strstr(args.c_str(), "%s");
+  if (inputPos == nullptr && Prior == nullptr) {
+    return FileRunCommandResult::Error("Only supported patterns are input file as argument or prior "
+      "command with disassembly");
+  }
+
+  CComPtr<IDxcLibrary> pLibrary;
+  CComPtr<IDxcBlobEncoding> pSource;
+  CComPtr<IDxcAssembler> pAssembler;
+  CComPtr<IDxcOperationResult> pResult;
+  CComPtr<IDxcContainerReflection> containerReflection;
+  uint32_t partCount;
+  CComPtr<IDxcBlob> pContainerBlob;
+  HRESULT resultStatus;
+  std::ostringstream ss;
+
+  IFT(DllSupport.CreateInstance(CLSID_DxcLibrary, &pLibrary));
+  IFT(DllSupport.CreateInstance(CLSID_DxcAssembler, &pAssembler));
+
+  if (inputPos != nullptr) {
+    args.erase(inputPos - args.c_str(), strlen("%s"));
+    IFT(pLibrary->CreateBlobFromFile(CommandFileName, nullptr, &pSource));
+  }
+  else {
+    assert(Prior != nullptr && "else early check should have returned");
+    CComPtr<IDxcAssembler> pAssembler;
+    IFT(DllSupport.CreateInstance(CLSID_DxcAssembler, &pAssembler));
+    IFT(pLibrary->CreateBlobWithEncodingFromPinned(
+        Prior->StdOut.c_str(), Prior->StdOut.size(), CP_UTF8,
+        &pSource));
+  }
+
+  IFT(pAssembler->AssembleToContainer(pSource, &pResult));
+  IFT(pResult->GetStatus(&resultStatus));
+  if (FAILED(resultStatus)) {
+    CComPtr<IDxcBlobEncoding> pAssembleBlob;
+    IFT(pResult->GetErrorBuffer(&pAssembleBlob));
+    return FileRunCommandResult::Error(resultStatus, BlobToUtf8(pAssembleBlob));
+  }
+  IFT(pResult->GetResult(&pContainerBlob));
+
+  IFT(DllSupport.CreateInstance(CLSID_DxcContainerReflection, &containerReflection));
+  IFT(containerReflection->Load(pContainerBlob));
+  IFT(containerReflection->GetPartCount(&partCount));
+
+  ss << "Part count: " << partCount << "\n";
+
+  for (uint32_t i = 0; i < partCount; ++i) {
+    uint32_t kind;
+    IFT(containerReflection->GetPartKind(i, &kind));
+    CComPtr<IDxcBlob> pPart;
+    IFT(containerReflection->GetPartContent(i, &pPart));
+    // #0 - SFI0 (8 bytes)
+    ss << "#" << i << " - ";
+    ss << (char)(kind & 0xff) << (char)((kind >> 8) & 0xff) << (char)((kind >> 16) & 0xff) << (char)((kind >> 24) & 0xff);
+    ss << " (" << pPart->GetBufferSize() << " bytes)\n";
+  }
+
+  ss.flush();
+
+  return FileRunCommandResult::Success(ss.str());
+}
+
 FileRunCommandResult FileRunCommandPart::RunD3DReflect(dxc::DxcDllSupport &DllSupport, const FileRunCommandResult *Prior) {
   std::string args(strtrim(Arguments));
   if (args != "%s")