Skip to content
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.

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

[DX] Add support for program signatures #67346

Merged
merged 4 commits into from
Oct 5, 2023

Conversation

llvm-beanz
Copy link
Collaborator

@llvm-beanz llvm-beanz commented Sep 25, 2023

For DirectX, program signatures are encoded into three different binary sections depending on if the signature is for inputs, outputs, or patches. All three signature types use the same data structure encoding so they can share a lot of logic.

This patch adds support for reading and writing program signature data as both yaml and binary data.

Fixes #57743 and #57744

For DirectX, program signatures are encoded into three different binary
sections depending on if the signature is for inputs, outputs, or
patches. All three signature types use the same data structure encoding
so they can share a lot of logic.

This patch adds support for reading and writing program signature data
as both yaml and binary data.
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 25, 2023

@llvm/pr-subscribers-mc
@llvm/pr-subscribers-llvm-binary-utilities
@llvm/pr-subscribers-objectyaml

@llvm/pr-subscribers-backend-directx

Changes

For DirectX, program signatures are encoded into three different binary sections depending on if the signature is for inputs, outputs, or patches. All three signature types use the same data structure encoding so they can share a lot of logic.

This patch adds support for reading and writing program signature data as both yaml and binary data.


Patch is 35.02 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/67346.diff

12 Files Affected:

  • (modified) llvm/include/llvm/BinaryFormat/DXContainer.h (+68)
  • (modified) llvm/include/llvm/BinaryFormat/DXContainerConstants.def (+51)
  • (modified) llvm/include/llvm/MC/DXContainerPSVInfo.h (+28)
  • (modified) llvm/include/llvm/Object/DXContainer.h (+108-65)
  • (modified) llvm/include/llvm/ObjectYAML/DXContainerYAML.h (+29)
  • (modified) llvm/lib/BinaryFormat/DXContainer.cpp (+30)
  • (modified) llvm/lib/MC/DXContainerPSVInfo.cpp (+48)
  • (modified) llvm/lib/Object/DXContainer.cpp (+24)
  • (modified) llvm/lib/ObjectYAML/DXContainerEmitter.cpp (+14)
  • (modified) llvm/lib/ObjectYAML/DXContainerYAML.cpp (+37)
  • (added) llvm/test/ObjectYAML/DXContainer/SignatureParts.yaml (+254)
  • (modified) llvm/tools/obj2yaml/dxcontainer2yaml.cpp (+19)
diff --git a/llvm/include/llvm/BinaryFormat/DXContainer.h b/llvm/include/llvm/BinaryFormat/DXContainer.h
index f92071e32222e1c..5d38a939202c10e 100644
--- a/llvm/include/llvm/BinaryFormat/DXContainer.h
+++ b/llvm/include/llvm/BinaryFormat/DXContainer.h
@@ -426,6 +426,74 @@ struct ResourceBindInfo : public v0::ResourceBindInfo {
 } // namespace v2
 } // namespace PSV
 
+#define COMPONENT_PRECISION(Val, Enum) Enum = Val,
+enum class SigMinPrecision : uint32_t {
+#include "DXContainerConstants.def"
+};
+
+ArrayRef<EnumEntry<SigMinPrecision>> getSigMinPrecisions();
+
+#define D3D_SYSTEM_VALUE(Val, Enum) Enum = Val,
+enum class D3DSystemValue : uint32_t {
+#include "DXContainerConstants.def"
+};
+
+ArrayRef<EnumEntry<D3DSystemValue>> getD3DSystemValues();
+
+#define COMPONENT_TYPE(Val, Enum) Enum = Val,
+enum class SigComponentType : uint32_t {
+#include "DXContainerConstants.def"
+};
+
+ArrayRef<EnumEntry<SigComponentType>> getSigComponentTypes();
+
+struct ProgramSignatureHeader {
+  uint32_t ParamCount;
+  uint32_t FirstParamOffset;
+
+  void swapBytes() {
+    sys::swapByteOrder(ParamCount);
+    sys::swapByteOrder(ParamCount);
+  }
+};
+
+struct ProgramSignatureElement {
+  uint32_t Stream;     // Stream index (parameters must appear in non-decreasing
+                       // stream order)
+  uint32_t NameOffset; // Offset to LPCSTR from start of ProgramSignatureHeader.
+  uint32_t Index;      // Semantic Index
+  D3DSystemValue SystemValue; // Semantic type. Similar to PSV::SemanticKind.
+  SigComponentType CompType;  // Type of bits.
+  uint32_t Register;          // Register Index (row index)
+  uint8_t Mask;               // Mask (column allocation)
+
+  // The ExclusiveMask has a different meaning for input and output signatures.
+  // For an output signature, masked components of the output register are never
+  // written to.
+  // For an input signature, masked components of the input register are always
+  // read.
+  uint8_t ExclusiveMask;
+
+  uint16_t Unused;
+  SigMinPrecision MinPrecision; // Minimum precision of input/output data
+
+  void swapBytes() {
+    sys::swapByteOrder(Stream);
+    sys::swapByteOrder(NameOffset);
+    sys::swapByteOrder(Index);
+    sys::swapByteOrder(SystemValue);
+    sys::swapByteOrder(CompType);
+    sys::swapByteOrder(Register);
+    sys::swapByteOrder(Mask);
+    sys::swapByteOrder(ExclusiveMask);
+    sys::swapByteOrder(MinPrecision);
+  }
+};
+
+// Easy to get this wrong. Earlier assertions can help determine
+static_assert(sizeof(ProgramSignatureElement) == 32,
+              "ProgramSignatureElement is misaligned");
+
 } // namespace dxbc
 } // namespace llvm
 
diff --git a/llvm/include/llvm/BinaryFormat/DXContainerConstants.def b/llvm/include/llvm/BinaryFormat/DXContainerConstants.def
index e2cbad293e16252..87dd0a5cb6ba709 100644
--- a/llvm/include/llvm/BinaryFormat/DXContainerConstants.def
+++ b/llvm/include/llvm/BinaryFormat/DXContainerConstants.def
@@ -4,6 +4,9 @@ CONTAINER_PART(DXIL)
 CONTAINER_PART(SFI0)
 CONTAINER_PART(HASH)
 CONTAINER_PART(PSV0)
+CONTAINER_PART(ISG1)
+CONTAINER_PART(OSG1)
+CONTAINER_PART(PSG1)
 
 #undef CONTAINER_PART
 #endif 
@@ -101,6 +104,20 @@ COMPONENT_TYPE(9, Float64)
 #undef COMPONENT_TYPE
 #endif
 
+#ifdef COMPONENT_PRECISION
+
+COMPONENT_PRECISION(0, Default)
+COMPONENT_PRECISION(1, Float16)
+COMPONENT_PRECISION(2, Float2_8)
+COMPONENT_PRECISION(3, Reserved)
+COMPONENT_PRECISION(4, SInt16)
+COMPONENT_PRECISION(5, UInt16)
+COMPONENT_PRECISION(0xf0, Any16)
+COMPONENT_PRECISION(0xf1, Any10)
+
+#undef COMPONENT_PRECISION
+#endif
+
 #ifdef INTERPOLATION_MODE
 
 INTERPOLATION_MODE(0, Undefined)
@@ -115,3 +132,37 @@ INTERPOLATION_MODE(8, Invalid)
 
 #undef INTERPOLATION_MODE
 #endif
+
+#ifdef D3D_SYSTEM_VALUE
+
+D3D_SYSTEM_VALUE(0, Undefined)
+D3D_SYSTEM_VALUE(1, Position)
+D3D_SYSTEM_VALUE(2, ClipDistance)
+D3D_SYSTEM_VALUE(3, CullDistance)
+D3D_SYSTEM_VALUE(4, RenderTargetArrayIndex)
+D3D_SYSTEM_VALUE(5, ViewPortArrayIndex)
+D3D_SYSTEM_VALUE(6, VertexID)
+D3D_SYSTEM_VALUE(7, PrimitiveID)
+D3D_SYSTEM_VALUE(8, InstanceID)
+D3D_SYSTEM_VALUE(9, IsFrontFace)
+D3D_SYSTEM_VALUE(10, SampleIndex)
+D3D_SYSTEM_VALUE(11, FinalQuadEdgeTessfactor)
+D3D_SYSTEM_VALUE(12, FinalQuadInsideTessfactor)
+D3D_SYSTEM_VALUE(13, FinalTriEdgeTessfactor)
+D3D_SYSTEM_VALUE(14, FinalTriInsideTessfactor)
+D3D_SYSTEM_VALUE(15, FinalLineDetailTessfactor)
+D3D_SYSTEM_VALUE(16, FinalLineDensityTessfactor)
+D3D_SYSTEM_VALUE(23, Barycentrics)
+D3D_SYSTEM_VALUE(24, ShadingRate)
+D3D_SYSTEM_VALUE(25, CullPrimitive)
+D3D_SYSTEM_VALUE(64, Target)
+D3D_SYSTEM_VALUE(65, Depth)
+D3D_SYSTEM_VALUE(66, Coverage)
+D3D_SYSTEM_VALUE(67, DepthGE)
+D3D_SYSTEM_VALUE(68, DepthLE)
+D3D_SYSTEM_VALUE(69, StencilRef)
+D3D_SYSTEM_VALUE(70, InnerCoverage)
+
+#undef D3D_SYSTEM_VALUE
+
+#endif
diff --git a/llvm/include/llvm/MC/DXContainerPSVInfo.h b/llvm/include/llvm/MC/DXContainerPSVInfo.h
index 9b1892088142b63..7d21c18d252f1cc 100644
--- a/llvm/include/llvm/MC/DXContainerPSVInfo.h
+++ b/llvm/include/llvm/MC/DXContainerPSVInfo.h
@@ -86,6 +86,34 @@ struct PSVRuntimeInfo {
   }
 };
 
+class Signature {
+  struct Parameter {
+    uint32_t Stream;
+    StringRef Name;
+    uint32_t Index;
+    dxbc::D3DSystemValue SystemValue;
+    dxbc::SigComponentType CompType;
+    uint32_t Register;
+    uint8_t Mask;
+    uint8_t ExclusiveMask;
+    dxbc::SigMinPrecision MinPrecision;
+  };
+
+  SmallVector<Parameter> Params;
+
+public:
+  void addParam(uint32_t Stream, StringRef Name, uint32_t Index,
+                dxbc::D3DSystemValue SystemValue,
+                dxbc::SigComponentType CompType, uint32_t Register,
+                uint8_t Mask, uint8_t ExclusiveMask,
+                dxbc::SigMinPrecision MinPrecision) {
+    Params.push_back(Parameter{Stream, Name, Index, SystemValue, CompType,
+                               Register, Mask, ExclusiveMask, MinPrecision});
+  }
+
+  void write(raw_ostream &OS);
+};
+
 } // namespace mcdxbc
 } // namespace llvm
 
diff --git a/llvm/include/llvm/Object/DXContainer.h b/llvm/include/llvm/Object/DXContainer.h
index 8f7ab59d7f38288..2caca6a9ae73dc0 100644
--- a/llvm/include/llvm/Object/DXContainer.h
+++ b/llvm/include/llvm/Object/DXContainer.h
@@ -27,8 +27,6 @@
 namespace llvm {
 namespace object {
 
-namespace DirectX {
-
 namespace detail {
 template <typename T>
 std::enable_if_t<std::is_arithmetic<T>::value, void> swapBytes(T &value) {
@@ -40,82 +38,85 @@ std::enable_if_t<std::is_class<T>::value, void> swapBytes(T &value) {
   value.swapBytes();
 }
 } // namespace detail
-class PSVRuntimeInfo {
 
-  // This class provides a view into the underlying resource array. The Resource
-  // data is little-endian encoded and may not be properly aligned to read
-  // directly from. The dereference operator creates a copy of the data and byte
-  // swaps it as appropriate.
-  template <typename T> struct ViewArray {
-    StringRef Data;
-    uint32_t Stride = sizeof(T); // size of each element in the list.
+// This class provides a view into the underlying resource array. The Resource
+// data is little-endian encoded and may not be properly aligned to read
+// directly from. The dereference operator creates a copy of the data and byte
+// swaps it as appropriate.
+template <typename T> struct ViewArray {
+  StringRef Data;
+  uint32_t Stride = sizeof(T); // size of each element in the list.
 
-    ViewArray() = default;
-    ViewArray(StringRef D, size_t S) : Data(D), Stride(S) {}
+  ViewArray() = default;
+  ViewArray(StringRef D, size_t S) : Data(D), Stride(S) {}
 
-    using value_type = T;
-    static constexpr uint32_t MaxStride() {
-      return static_cast<uint32_t>(sizeof(value_type));
-    }
+  using value_type = T;
+  static constexpr uint32_t MaxStride() {
+    return static_cast<uint32_t>(sizeof(value_type));
+  }
 
-    struct iterator {
-      StringRef Data;
-      uint32_t Stride; // size of each element in the list.
-      const char *Current;
-
-      iterator(const ViewArray &A, const char *C)
-          : Data(A.Data), Stride(A.Stride), Current(C) {}
-      iterator(const iterator &) = default;
-
-      value_type operator*() {
-        // Explicitly zero the structure so that unused fields are zeroed. It is
-        // up to the user to know if the fields are used by verifying the PSV
-        // version.
-        value_type Val;
-        std::memset(&Val, 0, sizeof(value_type));
-        if (Current >= Data.end())
-          return Val;
-        memcpy(static_cast<void *>(&Val), Current,
-               std::min(Stride, MaxStride()));
-        if (sys::IsBigEndianHost)
-          detail::swapBytes(Val);
+  struct iterator {
+    StringRef Data;
+    uint32_t Stride; // size of each element in the list.
+    const char *Current;
+
+    iterator(const ViewArray &A, const char *C)
+        : Data(A.Data), Stride(A.Stride), Current(C) {}
+    iterator(const iterator &) = default;
+
+    value_type operator*() {
+      // Explicitly zero the structure so that unused fields are zeroed. It is
+      // up to the user to know if the fields are used by verifying the PSV
+      // version.
+      value_type Val;
+      std::memset(&Val, 0, sizeof(value_type));
+      if (Current >= Data.end())
         return Val;
-      }
+      memcpy(static_cast<void *>(&Val), Current, std::min(Stride, MaxStride()));
+      if (sys::IsBigEndianHost)
+        detail::swapBytes(Val);
+      return Val;
+    }
 
-      iterator operator++() {
-        if (Current < Data.end())
-          Current += Stride;
-        return *this;
-      }
+    iterator operator++() {
+      if (Current < Data.end())
+        Current += Stride;
+      return *this;
+    }
 
-      iterator operator++(int) {
-        iterator Tmp = *this;
-        ++*this;
-        return Tmp;
-      }
+    iterator operator++(int) {
+      iterator Tmp = *this;
+      ++*this;
+      return Tmp;
+    }
 
-      iterator operator--() {
-        if (Current > Data.begin())
-          Current -= Stride;
-        return *this;
-      }
+    iterator operator--() {
+      if (Current > Data.begin())
+        Current -= Stride;
+      return *this;
+    }
 
-      iterator operator--(int) {
-        iterator Tmp = *this;
-        --*this;
-        return Tmp;
-      }
+    iterator operator--(int) {
+      iterator Tmp = *this;
+      --*this;
+      return Tmp;
+    }
 
-      bool operator==(const iterator I) { return I.Current == Current; }
-      bool operator!=(const iterator I) { return !(*this == I); }
-    };
+    bool operator==(const iterator I) { return I.Current == Current; }
+    bool operator!=(const iterator I) { return !(*this == I); }
+  };
 
-    iterator begin() const { return iterator(*this, Data.begin()); }
+  iterator begin() const { return iterator(*this, Data.begin()); }
 
-    iterator end() const { return iterator(*this, Data.end()); }
+  iterator end() const { return iterator(*this, Data.end()); }
 
-    size_t size() const { return Data.size() / Stride; }
-  };
+  size_t size() const { return Data.size() / Stride; }
+
+  bool isEmpty() const { return Data.empty(); }
+};
+
+namespace DirectX {
+class PSVRuntimeInfo {
 
   using ResourceArray = ViewArray<dxbc::PSV::v2::ResourceBindInfo>;
   using SigElementArray = ViewArray<dxbc::PSV::v0::SignatureElement>;
@@ -232,6 +233,36 @@ class PSVRuntimeInfo {
   }
 };
 
+class Signature {
+  ViewArray<dxbc::ProgramSignatureElement> Parameters;
+  StringRef StringTable;
+
+public:
+  ViewArray<dxbc::ProgramSignatureElement>::iterator begin() const {
+    return Parameters.begin();
+  }
+
+  ViewArray<dxbc::ProgramSignatureElement>::iterator end() const {
+    return Parameters.end();
+  }
+
+  StringRef getName(uint32_t Idx) const {
+    if (Idx == 0)
+      return "";
+
+    // Name offests are from the start of the signature data, not from the start
+    // of the string table.
+    uint32_t TableOffset =
+        Idx - sizeof(dxbc::ProgramSignatureHeader) -
+        (sizeof(dxbc::ProgramSignatureElement) * Parameters.size());
+    return StringTable.slice(TableOffset, StringTable.find('\0', TableOffset));
+  }
+
+  bool isEmpty() const { return Parameters.isEmpty(); }
+
+  Error initialize(StringRef Part);
+};
+
 } // namespace DirectX
 
 class DXContainer {
@@ -248,6 +279,9 @@ class DXContainer {
   std::optional<uint64_t> ShaderFlags;
   std::optional<dxbc::ShaderHash> Hash;
   std::optional<DirectX::PSVRuntimeInfo> PSVInfo;
+  DirectX::Signature InputSignature;
+  DirectX::Signature OutputSignature;
+  DirectX::Signature PatchConstantSignature;
 
   Error parseHeader();
   Error parsePartOffsets();
@@ -255,6 +289,7 @@ class DXContainer {
   Error parseShaderFlags(StringRef Part);
   Error parseHash(StringRef Part);
   Error parsePSVInfo(StringRef Part);
+  Error parseSignature(StringRef Part, DirectX::Signature &Array);
   friend class PartIterator;
 
 public:
@@ -340,6 +375,14 @@ class DXContainer {
   const std::optional<DirectX::PSVRuntimeInfo> &getPSVInfo() const {
     return PSVInfo;
   };
+
+  const DirectX::Signature &getInputSignature() const { return InputSignature; }
+  const DirectX::Signature &getOutputSignature() const {
+    return OutputSignature;
+  }
+  const DirectX::Signature &getPatchConstantSignature() const {
+    return PatchConstantSignature;
+  }
 };
 
 } // namespace object
diff --git a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
index a896b1881ff34a9..cc3870bfabd76fc 100644
--- a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
+++ b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
@@ -129,6 +129,22 @@ struct PSVInfo {
   PSVInfo(const dxbc::PSV::v2::RuntimeInfo *P);
 };
 
+struct SignatureParameter {
+  uint32_t Stream;
+  std::string Name;
+  uint32_t Index;
+  dxbc::D3DSystemValue SystemValue;
+  dxbc::SigComponentType CompType;
+  uint32_t Register;
+  uint8_t Mask;
+  uint8_t ExclusiveMask;
+  dxbc::SigMinPrecision MinPrecision;
+};
+
+struct Signature {
+  llvm::SmallVector<SignatureParameter> Parameters;
+};
+
 struct Part {
   Part() = default;
   Part(std::string N, uint32_t S) : Name(N), Size(S) {}
@@ -138,6 +154,7 @@ struct Part {
   std::optional<ShaderFlags> Flags;
   std::optional<ShaderHash> Hash;
   std::optional<PSVInfo> Info;
+  std::optional<Signature> Signature;
 };
 
 struct Object {
@@ -152,9 +169,13 @@ LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DXContainerYAML::Part)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DXContainerYAML::ResourceBindInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DXContainerYAML::SignatureElement)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DXContainerYAML::PSVInfo::MaskVector)
+LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DXContainerYAML::SignatureParameter)
 LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::PSV::SemanticKind)
 LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::PSV::ComponentType)
 LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::PSV::InterpolationMode)
+LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::D3DSystemValue)
+LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::SigComponentType)
+LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::SigMinPrecision)
 
 namespace llvm {
 
@@ -202,6 +223,14 @@ template <> struct MappingTraits<DXContainerYAML::SignatureElement> {
   static void mapping(IO &IO, llvm::DXContainerYAML::SignatureElement &El);
 };
 
+template <> struct MappingTraits<DXContainerYAML::SignatureParameter> {
+  static void mapping(IO &IO, llvm::DXContainerYAML::SignatureParameter &El);
+};
+
+template <> struct MappingTraits<DXContainerYAML::Signature> {
+  static void mapping(IO &IO, llvm::DXContainerYAML::Signature &El);
+};
+
 } // namespace yaml
 
 } // namespace llvm
diff --git a/llvm/lib/BinaryFormat/DXContainer.cpp b/llvm/lib/BinaryFormat/DXContainer.cpp
index f6613f16fafef6b..9c0e657b069697a 100644
--- a/llvm/lib/BinaryFormat/DXContainer.cpp
+++ b/llvm/lib/BinaryFormat/DXContainer.cpp
@@ -30,6 +30,36 @@ bool ShaderHash::isPopulated() {
   return Flags > 0 || 0 != memcmp(&Digest, &Zeros, 16);
 }
 
+#define COMPONENT_PRECISION(Val, Enum) {#Enum, SigMinPrecision::Enum},
+
+static const EnumEntry<SigMinPrecision> SigMinPrecisionNames[] = {
+#include "llvm/BinaryFormat/DXContainerConstants.def"
+};
+
+ArrayRef<EnumEntry<SigMinPrecision>> dxbc::getSigMinPrecisions() {
+  return ArrayRef(SigMinPrecisionNames);
+}
+
+#define D3D_SYSTEM_VALUE(Val, Enum) {#Enum, D3DSystemValue::Enum},
+
+static const EnumEntry<D3DSystemValue> D3DSystemValueNames[] = {
+#include "llvm/BinaryFormat/DXContainerConstants.def"
+};
+
+ArrayRef<EnumEntry<D3DSystemValue>> dxbc::getD3DSystemValues() {
+  return ArrayRef(D3DSystemValueNames);
+}
+
+#define COMPONENT_TYPE(Val, Enum) {#Enum, SigComponentType::Enum},
+
+static const EnumEntry<SigComponentType> SigComponentTypes[] = {
+#include "llvm/BinaryFormat/DXContainerConstants.def"
+};
+
+ArrayRef<EnumEntry<SigComponentType>> dxbc::getSigComponentTypes() {
+  return ArrayRef(SigComponentTypes);
+}
+
 #define SEMANTIC_KIND(Val, Enum) {#Enum, PSV::SemanticKind::Enum},
 
 static const EnumEntry<PSV::SemanticKind> SemanticKindNames[] = {
diff --git a/llvm/lib/MC/DXContainerPSVInfo.cpp b/llvm/lib/MC/DXContainerPSVInfo.cpp
index 533659053c36f3d..bdc6f79a68c0c9a 100644
--- a/llvm/lib/MC/DXContainerPSVInfo.cpp
+++ b/llvm/lib/MC/DXContainerPSVInfo.cpp
@@ -161,3 +161,51 @@ void PSVRuntimeInfo::write(raw_ostream &OS, uint32_t Version) const {
   support::endian::write_array(OS, ArrayRef<uint32_t>(PatchOutputMap),
                                support::little);
 }
+
+void Signature::write(raw_ostream &OS) {
+  SmallVector<dxbc::ProgramSignatureElement> SigParams;
+  SigParams.reserve(Params.size());
+  StringTableBuilder StrTabBuilder((StringTableBuilder::DWARF));
+
+  // Name offsets are from the start of the part. Pre-calculate the offset to
+  // the start of the string table so that it can be added to the table offset.
+  uint32_t TableStart = sizeof(dxbc::ProgramSignatureHeader) +
+                        (sizeof(dxbc::ProgramSignatureElement) * Params.size());
+
+  for (const auto &P : Params) {
+    // zero out the data
+    dxbc::ProgramSignatureElement FinalElement;
+    memset(&FinalElement, 0, sizeof(dxbc::ProgramSignatureElement));
+    FinalElement.Stream = P.Stream;
+    FinalElement.NameOffset =
+        static_cast<uint32_t>(StrTabBuilder.add(P.Name)) + TableStart;
+    FinalElement.Index = P.Index;
+    FinalElement.SystemValue = P.SystemValue;
+    FinalElement.CompType = P.CompType;
+    FinalElement.Register = P.Register;
+    FinalElement.Mask = P.Mask;
+    FinalElement.ExclusiveMask = P.ExclusiveMask;
+    FinalElement.MinPrecision = P.MinPrecision;
+    SigParams.push_back(FinalElement);
+  }
+
+  StrTabBuilder.finalizeInOrder();
+  stable_sort(SigParams, [&](const dxbc::ProgramSignatureElement &L,
+                             const dxbc::ProgramSignatureElement R) {
+    return std::tie(L.Stream, L.Register, L.NameOffset) <
+           std::tie(R.Stream, R.Register, R.NameOffset);
+  });
+  if (sys::IsBigEndianHost)
+    for (auto &El : SigParams)
+      El.swapBytes();
+
+  dxbc::ProgramSignatureHeader Header = {static_cast<uint32_t>(Params.size()),
+                                         sizeof(dxbc::ProgramSignatureHeader)};
+  if (sys::IsBigEndianHost)
+    Header.swapBytes();
+  OS.write(reinterpret_cast<const char *>(&Header),
+           sizeof(dxbc::ProgramSignatureHeader));
+  OS.write(reinterpret_cast<const char *>(SigParams.data()),
+           sizeof(dxbc::ProgramSignatureElement) * SigParams.size());
+  StrTabBuilder.write(OS);
+}
diff --git a/llvm/lib/Object/DXContainer.cpp b/llvm/lib/Object/DXContainer.cpp
index a3071d2c1b72e97..511531943b4674d 100644
--- a/llvm/lib/Object/DXContainer.cpp
+++ b/llvm/lib/Object/DXContainer.cpp
@@ -101,6 +101,18 @@ Error DXContainer::parsePSVInfo(StringRef Part) {
   return Error::success();
 }
 
+Error DirectX::Signature::initialize(StringRef Part) {
+  dxbc::ProgramSignatureHeader SigHeader;
+  if (Error Err = readStruct(Part, Part.begin(), SigHeader))
+    return Err;
+  size_t Size = sizeof(dxbc::ProgramSignatureElement) * SigHeader.ParamCount;
+  if (Part.size() < Size + SigHeader.FirstParamOffset)
+    return parseFailed("Signature extends beyond the part boundary.");
+  Parameters.Data = Part.substr(SigHeader.FirstParamOffset, Size);
+  StringTable = Part.substr(SigHeader.FirstParamOffset + Size);
+  return Error::success();
+}
+
 Error DXContainer::parsePartOffsets() {
   uint32_t LastOffset =
       sizeof(dxbc::Header) + (Header.PartCount * sizeof(uint...
[truncated]

}
};

// Easy to get this wrong. Earlier assertions can help determine
Copy link
Contributor

Choose a reason for hiding this comment

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

Incomplete comment? Probably don't really need to explain the static assert here anyway.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Doh


StringRef getName(uint32_t Idx) const {
if (Idx == 0)
return "";
Copy link
Contributor

Choose a reason for hiding this comment

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

Could probably use a comment here - It's not obvious to me why an index of zero should be an empty name rather than some kind of error. I also don't think this path is tested.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a good catch, and a vestige from my first pass through. When I wrote this I was thinking that the string table was the same format as the other string tables in the file which follow unix convention starting with a null byte for empty strings.

It turns out, that's not the case here, these should never be 0 because the offsets are section based and the string table starts late in the section. I'll add proper error handling and tests for range checks on the name strings.

@github-actions
Copy link

github-actions bot commented Sep 26, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@bogner bogner left a comment

Choose a reason for hiding this comment

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

LGTM

// of the string table. The header encodes the start offset of the sting
// table, so we convert the offset here.
uint32_t TableOffset = Offset - StringTableOffset;
return StringTable.slice(TableOffset, StringTable.find('\0', TableOffset));
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if it matters too much, since we do the validation elsewhere and the result would just be an empty string, but should we maybe assert that Offset >= StringTableOffset?

@llvm-beanz llvm-beanz merged commit 9f87522 into llvm:main Oct 5, 2023
3 checks passed
jhuber6 added a commit that referenced this pull request Oct 5, 2023
Summary:
This caused a lot of bots to fail, fix this so I can compile again by
explicitly stating the associated struct.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[DirectX] DXContainer ISG1 part
3 participants