[NFC] Refactoring DXContainerYaml Root Parameter representation #138318

joaosaffran · 2025-05-02T18:07:10Z

This PR removes the union usage from DXContainerYaml Root Parameters representation, it uses variant instead.
closes: #139585

llvmbot · 2025-05-02T18:07:45Z

@llvm/pr-subscribers-objectyaml

@llvm/pr-subscribers-backend-directx

Author: None (joaosaffran)

Changes

This PR removes the union usage from DXContainerYaml Root Parameters representation, it uses variant instead.

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

4 Files Affected:

(modified) llvm/include/llvm/ObjectYAML/DXContainerYAML.h (+11-84)
(modified) llvm/lib/ObjectYAML/DXContainerEmitter.cpp (+22-19)
(modified) llvm/lib/ObjectYAML/DXContainerYAML.cpp (+41-23)
(modified) llvm/test/ObjectYAML/DXContainer/RootSignature-MultipleParameters.yaml (-11)

diff --git a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
index 738108cc3d1be..d8d025fe4294b 100644
--- a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
+++ b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
@@ -26,6 +26,7 @@
 #include <cstdint>
 #include <optional>
 #include <string>
+#include <variant>
 #include <vector>
 
 namespace llvm {
@@ -112,97 +113,23 @@ struct DescriptorTableYaml {
   SmallVector<DescriptorRangeYaml> Ranges;
 };
 
+
+using ParameterData = std::variant<
+RootConstantsYaml,
+RootDescriptorYaml,
+DescriptorTableYaml
+>;
+
 struct RootParameterYamlDesc {
   uint32_t Type;
   uint32_t Visibility;
   uint32_t Offset;
+  ParameterData Data;
+
   RootParameterYamlDesc(){};
   RootParameterYamlDesc(uint32_t T) : Type(T) {
-    switch (T) {
-
-    case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit):
-      Constants = RootConstantsYaml();
-      break;
-    case llvm::to_underlying(dxbc::RootParameterType::CBV):
-    case llvm::to_underlying(dxbc::RootParameterType::SRV):
-    case llvm::to_underlying(dxbc::RootParameterType::UAV):
-      Descriptor = RootDescriptorYaml();
-      break;
-    case llvm::to_underlying(dxbc::RootParameterType::DescriptorTable):
-      Table = DescriptorTableYaml();
-      break;
-    }
-  }
-
-  ~RootParameterYamlDesc() {
-    switch (Type) {
-
-    case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit):
-      Constants.~RootConstantsYaml();
-      break;
-    case llvm::to_underlying(dxbc::RootParameterType::CBV):
-    case llvm::to_underlying(dxbc::RootParameterType::SRV):
-    case llvm::to_underlying(dxbc::RootParameterType::UAV):
-      Descriptor.~RootDescriptorYaml();
-      break;
-    case llvm::to_underlying(dxbc::RootParameterType::DescriptorTable):
-      Table.~DescriptorTableYaml();
-      break;
-    }
+   
   }
-
-  RootParameterYamlDesc(const RootParameterYamlDesc &Other)
-      : Type(Other.Type), Visibility(Other.Visibility), Offset(Other.Offset) {
-    // Initialize the appropriate union member based on Type
-    switch (Type) {
-    case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit):
-      // Placement new to construct the union member
-      new (&Constants) RootConstantsYaml(Other.Constants);
-      break;
-    case llvm::to_underlying(dxbc::RootParameterType::CBV):
-    case llvm::to_underlying(dxbc::RootParameterType::SRV):
-    case llvm::to_underlying(dxbc::RootParameterType::UAV):
-      new (&Descriptor) RootDescriptorYaml(Other.Descriptor);
-      break;
-    case llvm::to_underlying(dxbc::RootParameterType::DescriptorTable):
-      new (&Table) DescriptorTableYaml(Other.Table);
-      break;
-    }
-  }
-
-  RootParameterYamlDesc &operator=(const RootParameterYamlDesc &other) {
-    if (this != &other) {
-      // First, destroy the current union member
-      this->~RootParameterYamlDesc();
-
-      // Copy the basic members
-      Type = other.Type;
-      Visibility = other.Visibility;
-      Offset = other.Offset;
-
-      // Initialize the new union member based on the Type from 'other'
-      switch (Type) {
-      case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit):
-        new (&Constants) RootConstantsYaml(other.Constants);
-        break;
-      case llvm::to_underlying(dxbc::RootParameterType::CBV):
-      case llvm::to_underlying(dxbc::RootParameterType::SRV):
-      case llvm::to_underlying(dxbc::RootParameterType::UAV):
-        new (&Descriptor) RootDescriptorYaml(other.Descriptor);
-        break;
-      case llvm::to_underlying(dxbc::RootParameterType::DescriptorTable):
-        new (&Table) DescriptorTableYaml(other.Table);
-        break;
-      }
-    }
-    return *this;
-  }
-
-  union {
-    RootConstantsYaml Constants;
-    RootDescriptorYaml Descriptor;
-    DescriptorTableYaml Table;
-  };
 };
 
 struct RootSignatureYamlDesc {
diff --git a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp
index 6336a42c8e4ae..c2c0188a959b0 100644
--- a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp
+++ b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp
@@ -15,11 +15,13 @@
 #include "llvm/BinaryFormat/DXContainer.h"
 #include "llvm/MC/DXContainerPSVInfo.h"
 #include "llvm/MC/DXContainerRootSignature.h"
+#include "llvm/ObjectYAML/DXContainerYAML.h"
 #include "llvm/ObjectYAML/ObjectYAML.h"
 #include "llvm/ObjectYAML/yaml2obj.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/raw_ostream.h"
+#include <variant>
 
 using namespace llvm;
 
@@ -278,33 +280,35 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
         auto Header = dxbc::RootParameterHeader{Param.Type, Param.Visibility,
                                                 Param.Offset};
 
-        switch (Param.Type) {
-        case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit):
+        if(std::holds_alternative<DXContainerYAML::RootConstantsYaml>(Param.Data)){
+          auto ConstantYaml = std::get<DXContainerYAML::RootConstantsYaml>(Param.Data);
+
           dxbc::RootConstants Constants;
-          Constants.Num32BitValues = Param.Constants.Num32BitValues;
-          Constants.RegisterSpace = Param.Constants.RegisterSpace;
-          Constants.ShaderRegister = Param.Constants.ShaderRegister;
+          Constants.Num32BitValues = ConstantYaml.Num32BitValues;
+          Constants.RegisterSpace = ConstantYaml.RegisterSpace;
+          Constants.ShaderRegister = ConstantYaml.ShaderRegister;
           RS.ParametersContainer.addParameter(Header, Constants);
-          break;
-        case llvm::to_underlying(dxbc::RootParameterType::SRV):
-        case llvm::to_underlying(dxbc::RootParameterType::UAV):
-        case llvm::to_underlying(dxbc::RootParameterType::CBV):
+        } else if (std::holds_alternative<DXContainerYAML::RootDescriptorYaml>(Param.Data)){
+          auto DescriptorYaml = std::get<DXContainerYAML::RootDescriptorYaml>(Param.Data);
+
           if (RS.Version == 1) {
             dxbc::RST0::v0::RootDescriptor Descriptor;
-            Descriptor.RegisterSpace = Param.Descriptor.RegisterSpace;
-            Descriptor.ShaderRegister = Param.Descriptor.ShaderRegister;
+            auto DescriptorYaml = std::get<DXContainerYAML::RootDescriptorYaml>(Param.Data);
+            Descriptor.RegisterSpace = DescriptorYaml.RegisterSpace;
+            Descriptor.ShaderRegister = DescriptorYaml.ShaderRegister;
             RS.ParametersContainer.addParameter(Header, Descriptor);
           } else {
             dxbc::RST0::v1::RootDescriptor Descriptor;
-            Descriptor.RegisterSpace = Param.Descriptor.RegisterSpace;
-            Descriptor.ShaderRegister = Param.Descriptor.ShaderRegister;
-            Descriptor.Flags = Param.Descriptor.getEncodedFlags();
+            Descriptor.RegisterSpace = DescriptorYaml.RegisterSpace;
+            Descriptor.ShaderRegister = DescriptorYaml.ShaderRegister;
+            Descriptor.Flags = DescriptorYaml.getEncodedFlags();
           RS.ParametersContainer.addParameter(Header, Descriptor);
           }
-          break;
-        case llvm::to_underlying(dxbc::RootParameterType::DescriptorTable): {
+        }else if (std::holds_alternative<DXContainerYAML::DescriptorTableYaml>(Param.Data)) {
           mcdxbc::DescriptorTable Table;
-          for (const auto &R : Param.Table.Ranges) {
+          auto TableYaml = std::get<DXContainerYAML::DescriptorTableYaml>(Param.Data);
+
+          for (const auto &R : TableYaml.Ranges) {
             if (RS.Version == 1) {
               dxbc::RST0::v0::DescriptorRange Range;
               Range.RangeType = R.RangeType;
@@ -327,8 +331,7 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
             }
           }
           RS.ParametersContainer.addParameter(Header, Table);
-        } break;
-        default:
+        } else {
           // Handling invalid parameter type edge case
           RS.ParametersContainer.addInfo(Header, -1);
         }
diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
index 528fef59b46e4..3fea73c0da447 100644
--- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp
+++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
@@ -76,10 +76,11 @@ DXContainerYAML::RootSignatureYamlDesc::create(
         return std::move(E);
 
       auto Constants = *ConstantsOrErr;
-
-      NewP.Constants.Num32BitValues = Constants.Num32BitValues;
-      NewP.Constants.ShaderRegister = Constants.ShaderRegister;
-      NewP.Constants.RegisterSpace = Constants.RegisterSpace;
+      RootConstantsYaml ConstantYaml;
+      ConstantYaml.Num32BitValues = Constants.Num32BitValues;
+      ConstantYaml.ShaderRegister = Constants.ShaderRegister;
+      ConstantYaml.RegisterSpace = Constants.RegisterSpace;
+      NewP.Data = ConstantYaml;
     } else if (auto *RDV =
                    dyn_cast<object::DirectX::RootDescriptorView>(&ParamView)) {
       llvm::Expected<dxbc::RST0::v1::RootDescriptor> DescriptorOrErr =
@@ -87,15 +88,17 @@ DXContainerYAML::RootSignatureYamlDesc::create(
       if (Error E = DescriptorOrErr.takeError())
         return std::move(E);
       auto Descriptor = *DescriptorOrErr;
-      NewP.Descriptor.ShaderRegister = Descriptor.ShaderRegister;
-      NewP.Descriptor.RegisterSpace = Descriptor.RegisterSpace;
+      RootDescriptorYaml YamlDescriptor;
+      YamlDescriptor.ShaderRegister = Descriptor.ShaderRegister;
+      YamlDescriptor.RegisterSpace = Descriptor.RegisterSpace;
       if (Version > 1) {
 #define ROOT_DESCRIPTOR_FLAG(Num, Val)                                         \
-  NewP.Descriptor.Val =                                                        \
+  YamlDescriptor.Val =                                                        \
       (Descriptor.Flags &                                                      \
        llvm::to_underlying(dxbc::RootDescriptorFlag::Val)) > 0;
 #include "llvm/BinaryFormat/DXContainerConstants.def"
       }
+      NewP.Data = YamlDescriptor;
     } else if (auto *TDV = dyn_cast<object::DirectX::DescriptorTableView<
                    dxbc::RST0::v0::DescriptorRange>>(&ParamView)) {
       llvm::Expected<
@@ -104,8 +107,9 @@ DXContainerYAML::RootSignatureYamlDesc::create(
       if (Error E = TableOrErr.takeError())
         return std::move(E);
       auto Table = *TableOrErr;
-      NewP.Table.NumRanges = Table.NumRanges;
-      NewP.Table.RangesOffset = Table.RangesOffset;
+      DescriptorTableYaml YamlTable;
+      YamlTable.NumRanges = Table.NumRanges;
+      YamlTable.RangesOffset = Table.RangesOffset;
 
       for (const auto &R : Table) {
         DescriptorRangeYaml NewR;
@@ -117,8 +121,9 @@ DXContainerYAML::RootSignatureYamlDesc::create(
         NewR.RegisterSpace = R.RegisterSpace;
         NewR.RangeType = R.RangeType;
 
-        NewP.Table.Ranges.push_back(NewR);
+        YamlTable.Ranges.push_back(NewR);
       }
+      NewP.Data = YamlTable;
     } else if (auto *TDV = dyn_cast<object::DirectX::DescriptorTableView<
                    dxbc::RST0::v1::DescriptorRange>>(&ParamView)) {
       llvm::Expected<
@@ -127,8 +132,9 @@ DXContainerYAML::RootSignatureYamlDesc::create(
       if (Error E = TableOrErr.takeError())
         return std::move(E);
       auto Table = *TableOrErr;
-      NewP.Table.NumRanges = Table.NumRanges;
-      NewP.Table.RangesOffset = Table.RangesOffset;
+      DescriptorTableYaml YamlTable;
+      YamlTable.NumRanges = Table.NumRanges;
+      YamlTable.RangesOffset = Table.RangesOffset;
 
       for (const auto &R : Table) {
         DescriptorRangeYaml NewR;
@@ -143,8 +149,9 @@ DXContainerYAML::RootSignatureYamlDesc::create(
   NewR.Val =                                                                   \
       (R.Flags & llvm::to_underlying(dxbc::DescriptorRangeFlag::Val)) > 0;
 #include "llvm/BinaryFormat/DXContainerConstants.def"
-        NewP.Table.Ranges.push_back(NewR);
+        YamlTable.Ranges.push_back(NewR);
       }
+      NewP.Data = YamlTable;
     }
 
     RootSigDesc.Parameters.push_back(NewP);
@@ -394,18 +401,29 @@ void MappingTraits<llvm::DXContainerYAML::RootParameterYamlDesc>::mapping(
   IO.mapRequired("ShaderVisibility", P.Visibility);
 
   switch (P.Type) {
-  case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit):
-    IO.mapRequired("Constants", P.Constants);
-    break;
+  case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): {
+    DXContainerYAML::RootConstantsYaml Constants;
+    if(IO.outputting())
+      Constants = std::get<DXContainerYAML::RootConstantsYaml>(P.Data);
+    IO.mapRequired("Constants", Constants);
+    P.Data = Constants;
+  } break;
   case llvm::to_underlying(dxbc::RootParameterType::CBV):
   case llvm::to_underlying(dxbc::RootParameterType::SRV):
-  case llvm::to_underlying(dxbc::RootParameterType::UAV):
-    IO.mapRequired("Descriptor", P.Descriptor);
-    break;
-  case llvm::to_underlying(dxbc::RootParameterType::DescriptorTable):
-    IO.mapRequired("Table", P.Table);
-    break;
-    break;
+  case llvm::to_underlying(dxbc::RootParameterType::UAV):{
+    DXContainerYAML::RootDescriptorYaml Descriptor;
+    if(IO.outputting())
+      Descriptor = std::get<DXContainerYAML::RootDescriptorYaml>(P.Data);
+    IO.mapRequired("Descriptor", Descriptor);
+    P.Data = Descriptor;
+  } break;
+  case llvm::to_underlying(dxbc::RootParameterType::DescriptorTable): {
+    DXContainerYAML::DescriptorTableYaml Table;
+    if(IO.outputting())
+    Table = std::get<DXContainerYAML::DescriptorTableYaml>(P.Data);
+    IO.mapRequired("Table", Table);
+    P.Data = Table;
+  } break;
   }
 }
 
diff --git a/llvm/test/ObjectYAML/DXContainer/RootSignature-MultipleParameters.yaml b/llvm/test/ObjectYAML/DXContainer/RootSignature-MultipleParameters.yaml
index d680bf09ab730..debb459c3944e 100644
--- a/llvm/test/ObjectYAML/DXContainer/RootSignature-MultipleParameters.yaml
+++ b/llvm/test/ObjectYAML/DXContainer/RootSignature-MultipleParameters.yaml
@@ -37,17 +37,6 @@ Parts:
           ShaderRegister: 31
           RegisterSpace: 32
           DATA_STATIC_WHILE_SET_AT_EXECUTE: true
-      - ParameterType: 0 # SRV
-        ShaderVisibility: 3 # Domain
-        Table:
-          NumRanges: 1
-          Ranges:
-            - RangeType: 0
-              NumDescriptors: 41
-              BaseShaderRegister: 42
-              RegisterSpace: 43
-              OffsetInDescriptorsFromTableStart: -1
-              DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS: true
       AllowInputAssemblerInputLayout: true
       DenyGeometryShaderRootAccess: true

llvm/test/ObjectYAML/DXContainer/RootSignature-MultipleParameters.yaml

llvm/lib/ObjectYAML/DXContainerEmitter.cpp

llvm/include/llvm/ObjectYAML/DXContainerYAML.h

llvm/lib/ObjectYAML/DXContainerEmitter.cpp

github-actions · 2025-05-22T19:15:16Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff HEAD~1 HEAD --extensions h,cpp -- llvm/include/llvm/ObjectYAML/DXContainerYAML.h llvm/lib/ObjectYAML/DXContainerEmitter.cpp llvm/lib/ObjectYAML/DXContainerYAML.cpp

View the diff from clang-format here.

diff --git a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
index 3af1da552..a21b7a5ce 100644
--- a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
+++ b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
@@ -95,7 +95,7 @@ struct RootParameterHeaderYaml {
   uint32_t Visibility;
   uint32_t Offset;
 
-  RootParameterHeaderYaml(){};
+  RootParameterHeaderYaml() {};
   RootParameterHeaderYaml(uint32_t T) : Type(T) {}
 };
 
@@ -103,7 +103,7 @@ struct RootParameterLocationYaml {
   RootParameterHeaderYaml Header;
   std::optional<size_t> IndexInSignature;
 
-  RootParameterLocationYaml(){};
+  RootParameterLocationYaml() {};
   explicit RootParameterLocationYaml(RootParameterHeaderYaml Header)
       : Header(Header) {}
 };

inbelic · 2025-05-22T21:33:19Z

llvm/lib/ObjectYAML/DXContainerYAML.cpp

+    if (IO.outputting())
+      Constants = std::get<DXContainerYAML::RootConstantsYaml>(P.Data);


What does this do? Is this actually a NFC?

DXContainerYaml handles both serialization and deserialization of yaml files. IO.outputting() is a way for me to know which one is happening. Since parameter data is using a variant, the yaml serialization doesn't have a native way of handling that. Therefore, this is a custom logic to read and write the correct parameters data to and from YAML. I think this is still a NFC, since I am just keeping the existing functionality.

I see. Should we only assign P.Data = Constants when we are not outputting then? Otherwise, it seems we would overwrite it with garbage.

bogner

Similarly to the discussion in #137284, I'm not really convinced that std::variant makes sense here. These two types (RootConstantsYaml and RootDescriptorYaml) are relatively small and similarly sized (12 bytes each), so a strongly typed union like std::variant seems fairly reasonable, but consider what we get once we add Descriptor Table handling - that object contains a SmallVector of objects that are 28 bytes large each, as well as a couple of other fields. Even if the SmallVector's inline storage is only a single element, that's on the order of 5 times the size of the other two variant types, which means most of the objects in the vector of ParameterData will have 4 times as much wasted space as they do actual data.

On the other hand, if we put something like a std::optional<size_t> IndexInSignature here, we can store each type of data contiguously, and it doesn't even really change the amount of complexity overall. The one thing that will be a bit messier is the mapping function for RootParameterYamlDesc, but we can use MappingContextTraits there in order to pass along the RootSignatureYamlDesc as a "Context" there, and then define some lazy accessor helpers in RootSignatureYamlDesc, like:

  template <typename T>
  T &getOrInsertImpl(RootParameterYamlDesc &ParamDesc,
                     SmallVectorImpl<T> &Container) {
    if (!ParamDesc.IndexInSignature) {
      ParamDesc.IndexInSignature = Container.size();
      Container.emplace_back();
    }
    return Container[*ParamDesc.IndexInSignature];
  }
  RootConstantsYaml &getOrInsertConstants(RootParameterYamlDesc &ParamDesc) {
    return getOrInsertImpl(ParamDesc, Constants);
  }
  RootDescriptorYaml &getOrInsertDescriptor(RootParameterYamlDesc &ParamDesc) {
    return getOrInsertImpl(ParamDesc, Descriptors);
  }

bogner · 2025-05-27T20:59:33Z

llvm/lib/ObjectYAML/DXContainerEmitter.cpp

+            if (!P.RootSignature.has_value())
+              continue;


Looks like a funny whitespace change here, please double check.

bogner · 2025-05-27T21:00:59Z

llvm/lib/ObjectYAML/DXContainerEmitter.cpp

+              auto Header = dxbc::RootParameterHeader{
+                  L.Header.Type, L.Header.Visibility, L.Header.Offset};


I think this would be clearer.

Suggested change

auto Header = dxbc::RootParameterHeader{

L.Header.Type, L.Header.Visibility, L.Header.Offset};

dxbc::RootParameterHeader Header{

L.Header.Type, L.Header.Visibility, L.Header.Offset};

bogner · 2025-05-27T21:01:33Z

llvm/lib/ObjectYAML/DXContainerEmitter.cpp

+                DXContainerYAML::RootConstantsYaml ConstantYaml =
+                    P.RootSignature->Parameters.getOrInsertConstants(L);


Why make a copy here? This should be a reference.

inbelic · 2025-05-27T23:22:36Z

FYI this didn't pass the code formatter before merging

…#138318) This PR removes the union usage from `DXContainerYaml` Root Parameters representation, it uses variant instead. closes: [llvm#139585](llvm#139585) --------- Co-authored-by: joaosaffran <joao.saffran@microsoft.com>

joaosaffran added 18 commits April 24, 2025 22:06

adding support for Root Descriptors

0abacfc

clean up

8b8c02a

addressing comments

7ac9641

formating

c105458

try fix test

efe76aa

addressing comments

a928e9d

refactoring mcdxbc struct to store root parameters out of order

a38f10b

changing name

9a7c359

changing variant to host pointers

d6c2b55

clean up

93e4cf2

fix

b45b1b6

fix

f804a23

making read work

44bd13a

adding reading logic

ac51bf6

test pass locally

97fb003

adding tests

93e04bd

adding more tests

2f6d579

clean up

76b1b75

joaosaffran requested a review from damyanp May 2, 2025 18:07

joaosaffran assigned bogner May 2, 2025

llvmbot added backend:DirectX objectyaml labels May 2, 2025

joaosaffran assigned inbelic May 2, 2025

damyanp reviewed May 2, 2025

View reviewed changes

llvm/test/ObjectYAML/DXContainer/RootSignature-MultipleParameters.yaml Outdated Show resolved Hide resolved

damyanp reviewed May 2, 2025

View reviewed changes

llvm/lib/ObjectYAML/DXContainerEmitter.cpp Outdated Show resolved Hide resolved

damyanp reviewed May 2, 2025

View reviewed changes

llvm/lib/ObjectYAML/DXContainerEmitter.cpp Outdated Show resolved Hide resolved

joaosaffran added 3 commits May 2, 2025 18:42

refactoring root signature dxcontainer yaml representation

b2bfb02

clean up

9ee2964

fix test

2527580

inbelic reviewed May 20, 2025

View reviewed changes

llvm/include/llvm/ObjectYAML/DXContainerYAML.h Outdated Show resolved Hide resolved

llvm/lib/ObjectYAML/DXContainerEmitter.cpp Outdated Show resolved Hide resolved

joaosaffran marked this pull request as draft May 22, 2025 18:37

joaosaffran changed the base branch from users/joaosaffran/138315 to main May 22, 2025 18:37

joaosaffran added 2 commits May 22, 2025 18:58

Merge branch 'main' into refactoring/dxcontainer-yaml

df194b0

fix testing issues

0136cfc

joaosaffran added 3 commits May 22, 2025 19:26

clean up

b589d10

clean up

c7042b2

addressing PR Comments

70a9b7f

joaosaffran mentioned this pull request May 22, 2025

[DirectX] adding support to read/write descriptor table data using obj2yaml/yaml2obj #138315

Merged

joaosaffran marked this pull request as ready for review May 22, 2025 19:33

joaosaffran requested review from inbelic and damyanp May 22, 2025 19:33

formating

f1dd0ce

inbelic reviewed May 22, 2025

View reviewed changes

addressing comments

aabd424

joaosaffran force-pushed the refactoring/dxcontainer-yaml branch from e655315 to aabd424 Compare May 23, 2025 01:04

joaosaffran requested a review from inbelic May 23, 2025 01:05

inbelic approved these changes May 23, 2025

View reviewed changes

bogner requested changes May 23, 2025

View reviewed changes

addressing commets

e8ab8f7

joaosaffran requested a review from bogner May 24, 2025 05:54

clean up

bf5e7d3

bogner reviewed May 27, 2025

View reviewed changes

address pr comments

ea57253

joaosaffran requested a review from bogner May 27, 2025 21:57

bogner approved these changes May 27, 2025

View reviewed changes

joaosaffran merged commit 8c65a9d into llvm:main May 27, 2025
7 of 10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[NFC] Refactoring DXContainerYaml Root Parameter representation #138318

[NFC] Refactoring DXContainerYaml Root Parameter representation #138318

Uh oh!

joaosaffran commented May 2, 2025 •

edited

Loading

Uh oh!

llvmbot commented May 2, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented May 22, 2025 •

edited

Loading

Uh oh!

inbelic May 22, 2025

Uh oh!

joaosaffran May 22, 2025

Uh oh!

inbelic May 23, 2025 •

edited

Loading

Uh oh!

bogner left a comment

Uh oh!

bogner May 27, 2025

Uh oh!

bogner May 27, 2025

Uh oh!

bogner May 27, 2025

Uh oh!

Uh oh!

inbelic commented May 27, 2025

Uh oh!

Uh oh!

		if (IO.outputting())
		Constants = std::get<DXContainerYAML::RootConstantsYaml>(P.Data);

		auto Header = dxbc::RootParameterHeader{
		L.Header.Type, L.Header.Visibility, L.Header.Offset};

		DXContainerYAML::RootConstantsYaml ConstantYaml =
		P.RootSignature->Parameters.getOrInsertConstants(L);

[NFC] Refactoring DXContainerYaml Root Parameter representation #138318

[NFC] Refactoring DXContainerYaml Root Parameter representation #138318

Uh oh!

Conversation

joaosaffran commented May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

inbelic May 22, 2025

Choose a reason for hiding this comment

Uh oh!

joaosaffran May 22, 2025

Choose a reason for hiding this comment

Uh oh!

inbelic May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bogner left a comment

Choose a reason for hiding this comment

Uh oh!

bogner May 27, 2025

Choose a reason for hiding this comment

Uh oh!

bogner May 27, 2025

Choose a reason for hiding this comment

Uh oh!

bogner May 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

inbelic commented May 27, 2025

Uh oh!

Uh oh!

joaosaffran commented May 2, 2025 •

edited

Loading

llvmbot commented May 2, 2025 •

edited

Loading

github-actions bot commented May 22, 2025 •

edited

Loading

inbelic May 23, 2025 •

edited

Loading