[llvm] [DirectX] Fix order of `v2::DescriptorRange` (PR #145555)

via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 24 10:24:35 PDT 2025


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mc

Author: None (joaosaffran)

<details>
<summary>Changes</summary>

As pointed in #<!-- -->145438, the order of elements in `v2::DescriptorRange` is wrong according to the root signature file format. This changes the order and updates the code and test to continue to pass. 

Closes: #<!-- -->145438

---
Full diff: https://github.com/llvm/llvm-project/pull/145555.diff


6 Files Affected:

- (modified) llvm/include/llvm/BinaryFormat/DXContainer.h (+11-2) 
- (modified) llvm/include/llvm/Object/DXContainer.h (+7-14) 
- (modified) llvm/lib/MC/DXContainerRootSignature.cpp (+2-2) 
- (modified) llvm/lib/ObjectYAML/DXContainerYAML.cpp (+49-28) 
- (modified) llvm/unittests/Object/DXContainerTest.cpp (+4-4) 
- (modified) llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp (+2-2) 


``````````diff
diff --git a/llvm/include/llvm/BinaryFormat/DXContainer.h b/llvm/include/llvm/BinaryFormat/DXContainer.h
index 56c9e53308674..960720858f21b 100644
--- a/llvm/include/llvm/BinaryFormat/DXContainer.h
+++ b/llvm/include/llvm/BinaryFormat/DXContainer.h
@@ -740,10 +740,19 @@ struct RootDescriptor : public v1::RootDescriptor {
   }
 };
 
-struct DescriptorRange : public v1::DescriptorRange {
+struct DescriptorRange {
+  uint32_t RangeType;
+  uint32_t NumDescriptors;
+  uint32_t BaseShaderRegister;
+  uint32_t RegisterSpace;
   uint32_t Flags;
+  uint32_t OffsetInDescriptorsFromTableStart;
   void swapBytes() {
-    v1::DescriptorRange::swapBytes();
+    sys::swapByteOrder(RangeType);
+    sys::swapByteOrder(NumDescriptors);
+    sys::swapByteOrder(BaseShaderRegister);
+    sys::swapByteOrder(RegisterSpace);
+    sys::swapByteOrder(OffsetInDescriptorsFromTableStart);
     sys::swapByteOrder(Flags);
   }
 };
diff --git a/llvm/include/llvm/Object/DXContainer.h b/llvm/include/llvm/Object/DXContainer.h
index 4417061da936c..0fd8333c2e448 100644
--- a/llvm/include/llvm/Object/DXContainer.h
+++ b/llvm/include/llvm/Object/DXContainer.h
@@ -178,19 +178,14 @@ struct RootDescriptorView : RootParameterView {
     return readParameter<dxbc::RTS0::v2::RootDescriptor>();
   }
 };
-
-struct DescriptorTable {
+template <typename T> struct DescriptorTable {
   uint32_t NumRanges;
   uint32_t RangesOffset;
-  ViewArray<dxbc::RTS0::v2::DescriptorRange> Ranges;
+  ViewArray<T> Ranges;
 
-  typename ViewArray<dxbc::RTS0::v2::DescriptorRange>::iterator begin() const {
-    return Ranges.begin();
-  }
+  typename ViewArray<T>::iterator begin() const { return Ranges.begin(); }
 
-  typename ViewArray<dxbc::RTS0::v2::DescriptorRange>::iterator end() const {
-    return Ranges.end();
-  }
+  typename ViewArray<T>::iterator end() const { return Ranges.end(); }
 };
 
 struct DescriptorTableView : RootParameterView {
@@ -200,9 +195,9 @@ struct DescriptorTableView : RootParameterView {
   }
 
   // Define a type alias to access the template parameter from inside classof
-  llvm::Expected<DescriptorTable> read(uint32_t Version) {
+  template <typename T> llvm::Expected<DescriptorTable<T>> read() {
     const char *Current = ParamData.begin();
-    DescriptorTable Table;
+    DescriptorTable<T> Table;
 
     Table.NumRanges =
         support::endian::read<uint32_t, llvm::endianness::little>(Current);
@@ -212,9 +207,7 @@ struct DescriptorTableView : RootParameterView {
         support::endian::read<uint32_t, llvm::endianness::little>(Current);
     Current += sizeof(uint32_t);
 
-    size_t RangeSize = sizeof(dxbc::RTS0::v1::DescriptorRange);
-    if (Version > 1)
-      RangeSize = sizeof(dxbc::RTS0::v2::DescriptorRange);
+    size_t RangeSize = sizeof(T);
 
     Table.Ranges.Stride = RangeSize;
     Table.Ranges.Data =
diff --git a/llvm/lib/MC/DXContainerRootSignature.cpp b/llvm/lib/MC/DXContainerRootSignature.cpp
index 6c71823a51f85..0d51d96cfc7bf 100644
--- a/llvm/lib/MC/DXContainerRootSignature.cpp
+++ b/llvm/lib/MC/DXContainerRootSignature.cpp
@@ -133,10 +133,10 @@ void RootSignatureDesc::write(raw_ostream &OS) const {
                                llvm::endianness::little);
         support::endian::write(BOS, Range.RegisterSpace,
                                llvm::endianness::little);
-        support::endian::write(BOS, Range.OffsetInDescriptorsFromTableStart,
-                               llvm::endianness::little);
         if (Version > 1)
           support::endian::write(BOS, Range.Flags, llvm::endianness::little);
+        support::endian::write(BOS, Range.OffsetInDescriptorsFromTableStart,
+                               llvm::endianness::little);
       }
       break;
     }
diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
index 3bca25f3fc6c6..bae837c0b9986 100644
--- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp
+++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
@@ -33,6 +33,39 @@ DXContainerYAML::ShaderFeatureFlags::ShaderFeatureFlags(uint64_t FlagData) {
 #include "llvm/BinaryFormat/DXContainerConstants.def"
 }
 
+template <typename T>
+static void
+addDescriptorRanges(DXContainerYAML::RootParameterHeaderYaml &Header,
+                    DXContainerYAML::RootSignatureYamlDesc &RootSigDesc,
+                    const object::DirectX::DescriptorTable<T> &Table) {
+
+  DXContainerYAML::RootParameterLocationYaml Location(Header);
+  DXContainerYAML::DescriptorTableYaml &TableYaml =
+      RootSigDesc.Parameters.getOrInsertTable(Location);
+  RootSigDesc.Parameters.insertLocation(Location);
+
+  TableYaml.NumRanges = Table.NumRanges;
+  TableYaml.RangesOffset = Table.RangesOffset;
+
+  for (const auto &R : Table.Ranges) {
+    DXContainerYAML::DescriptorRangeYaml NewR;
+    NewR.OffsetInDescriptorsFromTableStart =
+        R.OffsetInDescriptorsFromTableStart;
+    NewR.NumDescriptors = R.NumDescriptors;
+    NewR.BaseShaderRegister = R.BaseShaderRegister;
+    NewR.RegisterSpace = R.RegisterSpace;
+    NewR.RangeType = R.RangeType;
+    if constexpr (std::is_same_v<T, dxbc::RTS0::v2::DescriptorRange>) {
+      // Set all flag fields for v2
+#define DESCRIPTOR_RANGE_FLAG(Num, Val)                                        \
+  NewR.Val =                                                                   \
+      (R.Flags & llvm::to_underlying(dxbc::DescriptorRangeFlag::Val)) != 0;
+#include "llvm/BinaryFormat/DXContainerConstants.def"
+    }
+    TableYaml.Ranges.push_back(NewR);
+  }
+}
+
 llvm::Expected<DXContainerYAML::RootSignatureYamlDesc>
 DXContainerYAML::RootSignatureYamlDesc::create(
     const object::DirectX::RootSignature &Data) {
@@ -107,35 +140,23 @@ DXContainerYAML::RootSignatureYamlDesc::create(
       }
     } else if (auto *DTV =
                    dyn_cast<object::DirectX::DescriptorTableView>(&ParamView)) {
-      llvm::Expected<object::DirectX::DescriptorTable> TableOrErr =
-          DTV->read(Version);
-      if (Error E = TableOrErr.takeError())
-        return std::move(E);
-      auto Table = *TableOrErr;
-      RootParameterLocationYaml Location(Header);
-      DescriptorTableYaml &TableYaml =
-          RootSigDesc.Parameters.getOrInsertTable(Location);
-      RootSigDesc.Parameters.insertLocation(Location);
-
-      TableYaml.NumRanges = Table.NumRanges;
-      TableYaml.RangesOffset = Table.RangesOffset;
-
-      for (const auto &R : Table) {
-        DescriptorRangeYaml NewR;
 
-        NewR.OffsetInDescriptorsFromTableStart =
-            R.OffsetInDescriptorsFromTableStart;
-        NewR.NumDescriptors = R.NumDescriptors;
-        NewR.BaseShaderRegister = R.BaseShaderRegister;
-        NewR.RegisterSpace = R.RegisterSpace;
-        NewR.RangeType = R.RangeType;
-        if (Version > 1) {
-#define DESCRIPTOR_RANGE_FLAG(Num, Val)                                        \
-  NewR.Val =                                                                   \
-      (R.Flags & llvm::to_underlying(dxbc::DescriptorRangeFlag::Val)) > 0;
-#include "llvm/BinaryFormat/DXContainerConstants.def"
-        }
-        TableYaml.Ranges.push_back(NewR);
+      if (Version == 1) {
+        llvm::Expected<
+            object::DirectX::DescriptorTable<dxbc::RTS0::v1::DescriptorRange>>
+            TableOrErr = DTV->read<dxbc::RTS0::v1::DescriptorRange>();
+        if (Error E = TableOrErr.takeError())
+          return std::move(E);
+        auto Table = *TableOrErr;
+        addDescriptorRanges(Header, RootSigDesc, Table);
+      } else {
+        llvm::Expected<
+            object::DirectX::DescriptorTable<dxbc::RTS0::v2::DescriptorRange>>
+            TableOrErr = DTV->read<dxbc::RTS0::v2::DescriptorRange>();
+        if (Error E = TableOrErr.takeError())
+          return std::move(E);
+        auto Table = *TableOrErr;
+        addDescriptorRanges(Header, RootSigDesc, Table);
       }
     }
   }
diff --git a/llvm/unittests/Object/DXContainerTest.cpp b/llvm/unittests/Object/DXContainerTest.cpp
index 28012073d3c78..396d060a75bfd 100644
--- a/llvm/unittests/Object/DXContainerTest.cpp
+++ b/llvm/unittests/Object/DXContainerTest.cpp
@@ -1062,8 +1062,8 @@ TEST(RootSignature, ParseDescriptorTable) {
         0x3c, 0x00, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
         0x03, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
         0x2c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff,
-        0x2a, 0x00, 0x00, 0x00, 0x2b, 0x00, 0x00, 0x00, 0x29, 0x00, 0x00, 0x00,
-        0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x2a, 0x00, 0x00, 0x00, 0x2b, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00,
+        0x29, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
         0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
         0x00};
     DXContainer C =
@@ -1088,7 +1088,7 @@ TEST(RootSignature, ParseDescriptorTable) {
     auto *DescriptorTableView =
         dyn_cast<DirectX::DescriptorTableView>(&*ParamView);
     ASSERT_TRUE(DescriptorTableView != nullptr);
-    auto Table = DescriptorTableView->read(2);
+    auto Table = DescriptorTableView->read<dxbc::RTS0::v2::DescriptorRange>();
 
     ASSERT_THAT_ERROR(Table.takeError(), Succeeded());
 
@@ -1140,7 +1140,7 @@ TEST(RootSignature, ParseDescriptorTable) {
     auto *DescriptorTableView =
         dyn_cast<DirectX::DescriptorTableView>(&*ParamView);
     ASSERT_TRUE(DescriptorTableView != nullptr);
-    auto Table = DescriptorTableView->read(1);
+    auto Table = DescriptorTableView->read<dxbc::RTS0::v1::DescriptorRange>();
 
     ASSERT_THAT_ERROR(Table.takeError(), Succeeded());
 
diff --git a/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp b/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp
index 879712f833e13..9e6f7cc4f7dbd 100644
--- a/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp
+++ b/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp
@@ -459,8 +459,8 @@ TEST(RootSignature, ParseDescriptorTableV11) {
       0x3c, 0x00, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
       0x03, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
       0x2c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff,
-      0x2a, 0x00, 0x00, 0x00, 0x2b, 0x00, 0x00, 0x00, 0x29, 0x00, 0x00, 0x00,
-      0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+      0x2a, 0x00, 0x00, 0x00, 0x2b, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00,
+      0x29, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
       0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
       0x00};
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/145555


More information about the llvm-commits mailing list