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

via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 24 12:48:09 PDT 2025


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

>From 24cbe8b0bab2253b9189e3fc1a622dc3012477e1 Mon Sep 17 00:00:00 2001
From: joaosaffran <joao.saffran at microsoft.com>
Date: Tue, 24 Jun 2025 16:51:56 +0000
Subject: [PATCH 1/4] fix bug

---
 llvm/include/llvm/BinaryFormat/DXContainer.h  |  13 ++-
 llvm/include/llvm/Object/DXContainer.h        |  18 ++--
 llvm/lib/MC/DXContainerRootSignature.cpp      |   4 +-
 llvm/lib/ObjectYAML/DXContainerYAML.cpp       | 100 +++++++++++++-----
 llvm/unittests/Object/DXContainerTest.cpp     |   9 +-
 .../ObjectYAML/DXContainerYAMLTest.cpp        |   4 +-
 6 files changed, 99 insertions(+), 49 deletions(-)

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..4e031cbf633fc 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,10 @@ 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(uint32_t Version) {
     const char *Current = ParamData.begin();
-    DescriptorTable Table;
+    DescriptorTable<T> Table;
 
     Table.NumRanges =
         support::endian::read<uint32_t, llvm::endianness::little>(Current);
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..53efefaa0471c 100644
--- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp
+++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
@@ -33,6 +33,62 @@ DXContainerYAML::ShaderFeatureFlags::ShaderFeatureFlags(uint64_t FlagData) {
 #include "llvm/BinaryFormat/DXContainerConstants.def"
 }
 
+static void AddDescriptorRanges(
+    DXContainerYAML::RootParameterHeaderYaml &Header,
+    DXContainerYAML::RootSignatureYamlDesc &RootSigDesc,
+    object::DirectX::DescriptorTable<dxbc::RTS0::v1::DescriptorRange> &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) {
+    DXContainerYAML::DescriptorRangeYaml NewR;
+
+    NewR.OffsetInDescriptorsFromTableStart =
+        R.OffsetInDescriptorsFromTableStart;
+    NewR.NumDescriptors = R.NumDescriptors;
+    NewR.BaseShaderRegister = R.BaseShaderRegister;
+    NewR.RegisterSpace = R.RegisterSpace;
+    NewR.RangeType = R.RangeType;
+    TableYaml.Ranges.push_back(NewR);
+  }
+}
+
+static void AddDescriptorRanges(
+    DXContainerYAML::RootParameterHeaderYaml &Header,
+    DXContainerYAML::RootSignatureYamlDesc &RootSigDesc,
+    object::DirectX::DescriptorTable<dxbc::RTS0::v2::DescriptorRange> &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) {
+    DXContainerYAML::DescriptorRangeYaml NewR;
+
+    NewR.OffsetInDescriptorsFromTableStart =
+        R.OffsetInDescriptorsFromTableStart;
+    NewR.NumDescriptors = R.NumDescriptors;
+    NewR.BaseShaderRegister = R.BaseShaderRegister;
+    NewR.RegisterSpace = R.RegisterSpace;
+    NewR.RangeType = R.RangeType;
+#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 +163,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>(Version);
+        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>(Version);
+        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..29d0c429ff993 100644
--- a/llvm/unittests/Object/DXContainerTest.cpp
+++ b/llvm/unittests/Object/DXContainerTest.cpp
@@ -8,6 +8,7 @@
 
 #include "llvm/Object/DXContainer.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/BinaryFormat/DXContainer.h"
 #include "llvm/BinaryFormat/Magic.h"
 #include "llvm/ObjectYAML/DXContainerYAML.h"
 #include "llvm/ObjectYAML/yaml2obj.h"
@@ -1062,8 +1063,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 +1089,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>(2);
 
     ASSERT_THAT_ERROR(Table.takeError(), Succeeded());
 
@@ -1140,7 +1141,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>(1);
 
     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};
 

>From f5949b76464a4e949b9bff76c4d93b6324f7a386 Mon Sep 17 00:00:00 2001
From: joaosaffran <joao.saffran at microsoft.com>
Date: Tue, 24 Jun 2025 17:12:02 +0000
Subject: [PATCH 2/4] make code more llvmish

---
 llvm/include/llvm/Object/DXContainer.h    |  7 +---
 llvm/lib/ObjectYAML/DXContainerYAML.cpp   | 51 +++++++----------------
 llvm/unittests/Object/DXContainerTest.cpp |  4 +-
 3 files changed, 18 insertions(+), 44 deletions(-)

diff --git a/llvm/include/llvm/Object/DXContainer.h b/llvm/include/llvm/Object/DXContainer.h
index 4e031cbf633fc..0fd8333c2e448 100644
--- a/llvm/include/llvm/Object/DXContainer.h
+++ b/llvm/include/llvm/Object/DXContainer.h
@@ -195,8 +195,7 @@ struct DescriptorTableView : RootParameterView {
   }
 
   // Define a type alias to access the template parameter from inside classof
-  template <typename T>
-  llvm::Expected<DescriptorTable<T>> read(uint32_t Version) {
+  template <typename T> llvm::Expected<DescriptorTable<T>> read() {
     const char *Current = ParamData.begin();
     DescriptorTable<T> Table;
 
@@ -208,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/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
index 53efefaa0471c..bae837c0b9986 100644
--- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp
+++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
@@ -33,10 +33,11 @@ DXContainerYAML::ShaderFeatureFlags::ShaderFeatureFlags(uint64_t FlagData) {
 #include "llvm/BinaryFormat/DXContainerConstants.def"
 }
 
-static void AddDescriptorRanges(
-    DXContainerYAML::RootParameterHeaderYaml &Header,
-    DXContainerYAML::RootSignatureYamlDesc &RootSigDesc,
-    object::DirectX::DescriptorTable<dxbc::RTS0::v1::DescriptorRange> &Table) {
+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 =
@@ -46,45 +47,21 @@ static void AddDescriptorRanges(
   TableYaml.NumRanges = Table.NumRanges;
   TableYaml.RangesOffset = Table.RangesOffset;
 
-  for (const auto &R : Table) {
+  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;
-    TableYaml.Ranges.push_back(NewR);
-  }
-}
-
-static void AddDescriptorRanges(
-    DXContainerYAML::RootParameterHeaderYaml &Header,
-    DXContainerYAML::RootSignatureYamlDesc &RootSigDesc,
-    object::DirectX::DescriptorTable<dxbc::RTS0::v2::DescriptorRange> &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) {
-    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;
+      (R.Flags & llvm::to_underlying(dxbc::DescriptorRangeFlag::Val)) != 0;
 #include "llvm/BinaryFormat/DXContainerConstants.def"
+    }
     TableYaml.Ranges.push_back(NewR);
   }
 }
@@ -167,19 +144,19 @@ DXContainerYAML::RootSignatureYamlDesc::create(
       if (Version == 1) {
         llvm::Expected<
             object::DirectX::DescriptorTable<dxbc::RTS0::v1::DescriptorRange>>
-            TableOrErr = DTV->read<dxbc::RTS0::v1::DescriptorRange>(Version);
+            TableOrErr = DTV->read<dxbc::RTS0::v1::DescriptorRange>();
         if (Error E = TableOrErr.takeError())
           return std::move(E);
         auto Table = *TableOrErr;
-        AddDescriptorRanges(Header, RootSigDesc, Table);
+        addDescriptorRanges(Header, RootSigDesc, Table);
       } else {
         llvm::Expected<
             object::DirectX::DescriptorTable<dxbc::RTS0::v2::DescriptorRange>>
-            TableOrErr = DTV->read<dxbc::RTS0::v2::DescriptorRange>(Version);
+            TableOrErr = DTV->read<dxbc::RTS0::v2::DescriptorRange>();
         if (Error E = TableOrErr.takeError())
           return std::move(E);
         auto Table = *TableOrErr;
-        AddDescriptorRanges(Header, RootSigDesc, Table);
+        addDescriptorRanges(Header, RootSigDesc, Table);
       }
     }
   }
diff --git a/llvm/unittests/Object/DXContainerTest.cpp b/llvm/unittests/Object/DXContainerTest.cpp
index 29d0c429ff993..2723b4408e2ad 100644
--- a/llvm/unittests/Object/DXContainerTest.cpp
+++ b/llvm/unittests/Object/DXContainerTest.cpp
@@ -1089,7 +1089,7 @@ TEST(RootSignature, ParseDescriptorTable) {
     auto *DescriptorTableView =
         dyn_cast<DirectX::DescriptorTableView>(&*ParamView);
     ASSERT_TRUE(DescriptorTableView != nullptr);
-    auto Table = DescriptorTableView->read<dxbc::RTS0::v2::DescriptorRange>(2);
+    auto Table = DescriptorTableView->read<dxbc::RTS0::v2::DescriptorRange>();
 
     ASSERT_THAT_ERROR(Table.takeError(), Succeeded());
 
@@ -1141,7 +1141,7 @@ TEST(RootSignature, ParseDescriptorTable) {
     auto *DescriptorTableView =
         dyn_cast<DirectX::DescriptorTableView>(&*ParamView);
     ASSERT_TRUE(DescriptorTableView != nullptr);
-    auto Table = DescriptorTableView->read<dxbc::RTS0::v1::DescriptorRange>(1);
+    auto Table = DescriptorTableView->read<dxbc::RTS0::v1::DescriptorRange>();
 
     ASSERT_THAT_ERROR(Table.takeError(), Succeeded());
 

>From cc554045f4486c54b4358291c90f308839a34747 Mon Sep 17 00:00:00 2001
From: joaosaffran <joao.saffran at microsoft.com>
Date: Tue, 24 Jun 2025 17:20:12 +0000
Subject: [PATCH 3/4] clean up

---
 llvm/unittests/Object/DXContainerTest.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/llvm/unittests/Object/DXContainerTest.cpp b/llvm/unittests/Object/DXContainerTest.cpp
index 2723b4408e2ad..396d060a75bfd 100644
--- a/llvm/unittests/Object/DXContainerTest.cpp
+++ b/llvm/unittests/Object/DXContainerTest.cpp
@@ -8,7 +8,6 @@
 
 #include "llvm/Object/DXContainer.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/BinaryFormat/DXContainer.h"
 #include "llvm/BinaryFormat/Magic.h"
 #include "llvm/ObjectYAML/DXContainerYAML.h"
 #include "llvm/ObjectYAML/yaml2obj.h"

>From e86dc8e3e3fb6f08c7e41bf19ad1c53b1fd06513 Mon Sep 17 00:00:00 2001
From: joaosaffran <joao.saffran at microsoft.com>
Date: Tue, 24 Jun 2025 19:47:56 +0000
Subject: [PATCH 4/4] address comments

---
 llvm/include/llvm/Object/DXContainer.h  |  5 +---
 llvm/lib/ObjectYAML/DXContainerYAML.cpp | 38 ++++++++++++-------------
 2 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/llvm/include/llvm/Object/DXContainer.h b/llvm/include/llvm/Object/DXContainer.h
index 0fd8333c2e448..eb6b2ee816ac0 100644
--- a/llvm/include/llvm/Object/DXContainer.h
+++ b/llvm/include/llvm/Object/DXContainer.h
@@ -207,11 +207,8 @@ struct DescriptorTableView : RootParameterView {
         support::endian::read<uint32_t, llvm::endianness::little>(Current);
     Current += sizeof(uint32_t);
 
-    size_t RangeSize = sizeof(T);
-
-    Table.Ranges.Stride = RangeSize;
     Table.Ranges.Data =
-        ParamData.substr(2 * sizeof(uint32_t), Table.NumRanges * RangeSize);
+        ParamData.substr(2 * sizeof(uint32_t), Table.NumRanges * sizeof(T));
     return Table;
   }
 };
diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
index bae837c0b9986..f6d0e5227e64a 100644
--- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp
+++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
@@ -34,10 +34,16 @@ DXContainerYAML::ShaderFeatureFlags::ShaderFeatureFlags(uint64_t FlagData) {
 }
 
 template <typename T>
-static void
-addDescriptorRanges(DXContainerYAML::RootParameterHeaderYaml &Header,
-                    DXContainerYAML::RootSignatureYamlDesc &RootSigDesc,
-                    const object::DirectX::DescriptorTable<T> &Table) {
+static llvm::Error
+readDescriptorRanges(DXContainerYAML::RootParameterHeaderYaml &Header,
+                     DXContainerYAML::RootSignatureYamlDesc &RootSigDesc,
+                     object::DirectX::DescriptorTableView *DTV) {
+
+  llvm::Expected<object::DirectX::DescriptorTable<T>> TableOrErr =
+      DTV->read<T>();
+  if (Error E = TableOrErr.takeError())
+    return E;
+  auto Table = *TableOrErr;
 
   DXContainerYAML::RootParameterLocationYaml Location(Header);
   DXContainerYAML::DescriptorTableYaml &TableYaml =
@@ -64,6 +70,8 @@ addDescriptorRanges(DXContainerYAML::RootParameterHeaderYaml &Header,
     }
     TableYaml.Ranges.push_back(NewR);
   }
+
+  return Error::success();
 }
 
 llvm::Expected<DXContainerYAML::RootSignatureYamlDesc>
@@ -140,24 +148,16 @@ DXContainerYAML::RootSignatureYamlDesc::create(
       }
     } else if (auto *DTV =
                    dyn_cast<object::DirectX::DescriptorTableView>(&ParamView)) {
-
       if (Version == 1) {
-        llvm::Expected<
-            object::DirectX::DescriptorTable<dxbc::RTS0::v1::DescriptorRange>>
-            TableOrErr = DTV->read<dxbc::RTS0::v1::DescriptorRange>();
-        if (Error E = TableOrErr.takeError())
+        if (Error E = readDescriptorRanges<dxbc::RTS0::v1::DescriptorRange>(
+                Header, RootSigDesc, DTV))
           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())
+      } else if (Version == 2) {
+        if (Error E = readDescriptorRanges<dxbc::RTS0::v2::DescriptorRange>(
+                Header, RootSigDesc, DTV))
           return std::move(E);
-        auto Table = *TableOrErr;
-        addDescriptorRanges(Header, RootSigDesc, Table);
-      }
+      } else
+        llvm_unreachable("Unknown version for DescriptorRanges");
     }
   }
 



More information about the llvm-commits mailing list