addressing comments

Author: joaosaffran

llvm/include/llvm/BinaryFormat/DXContainer.h

@@ -602,7 +602,7 @@ struct RootDescriptor : public v1::RootDescriptor {
   uint32_t Flags;
 
   RootDescriptor() = default;
-  RootDescriptor(v1::RootDescriptor &Base)
+  explicit RootDescriptor(v1::RootDescriptor &Base)
       : v1::RootDescriptor(Base), Flags(0u) {}
 
   void swapBytes() {

llvm/include/llvm/MC/DXContainerRootSignature.h

@@ -21,8 +21,8 @@ struct RootParameterInfo {
 
   RootParameterInfo() = default;
 
-  RootParameterInfo(dxbc::RootParameterHeader H, size_t L)
-      : Header(H), Location(L) {}
+  RootParameterInfo(dxbc::RootParameterHeader Header, size_t Location)
+      : Header(Header), Location(Location) {}
 };
 
 struct RootParametersContainer {
@@ -31,29 +31,34 @@ struct RootParametersContainer {
   SmallVector<dxbc::RootConstants> Constants;
   SmallVector<dxbc::RTS0::v2::RootDescriptor> Descriptors;
 
-  void addInfo(dxbc::RootParameterHeader H, size_t L) {
-    ParametersInfo.push_back(RootParameterInfo(H, L));
+  void addInfo(dxbc::RootParameterHeader Header, size_t Location) {
+    ParametersInfo.push_back(RootParameterInfo(Header, Location));
   }
 
-  void addParameter(dxbc::RootParameterHeader H, dxbc::RootConstants C) {
-    addInfo(H, Constants.size());
-    Constants.push_back(C);
+  void addParameter(dxbc::RootParameterHeader Header,
+                    dxbc::RootConstants Constant) {
+    addInfo(Header, Constants.size());
+    Constants.push_back(Constant);
   }
 
-  void addParameter(dxbc::RootParameterHeader H,
-                    dxbc::RTS0::v2::RootDescriptor D) {
-    addInfo(H, Descriptors.size());
-    Descriptors.push_back(D);
+  void addInvalidParameter(dxbc::RootParameterHeader Header) {
+    addInfo(Header, -1);
+  }
+
+  void addParameter(dxbc::RootParameterHeader Header,
+                    dxbc::RTS0::v2::RootDescriptor Descriptor) {
+    addInfo(Header, Descriptors.size());
+    Descriptors.push_back(Descriptor);
   }
 
   const std::pair<uint32_t, uint32_t>
-  getTypeAndLocForParameter(uint32_t Index) const {
-    const RootParameterInfo &Info = ParametersInfo[Index];
+  getTypeAndLocForParameter(uint32_t Location) const {
+    const RootParameterInfo &Info = ParametersInfo[Location];
     return {Info.Header.ParameterType, Info.Location};
   }
 
-  const dxbc::RootParameterHeader &getHeader(size_t Index) const {
-    const RootParameterInfo &Info = ParametersInfo[Index];
+  const dxbc::RootParameterHeader &getHeader(size_t Location) const {
+    const RootParameterInfo &Info = ParametersInfo[Location];
     return Info.Header;
   }
 

llvm/lib/MC/DXContainerRootSignature.cpp

@@ -89,7 +89,8 @@ void RootSignatureDesc::write(raw_ostream &OS) const {
                              llvm::endianness::little);
       support::endian::write(BOS, Constants.Num32BitValues,
                              llvm::endianness::little);
-    } break;
+      break;
+    }
     case llvm::to_underlying(dxbc::RootParameterType::CBV):
     case llvm::to_underlying(dxbc::RootParameterType::SRV):
     case llvm::to_underlying(dxbc::RootParameterType::UAV): {
@@ -102,7 +103,8 @@ void RootSignatureDesc::write(raw_ostream &OS) const {
                              llvm::endianness::little);
       if (Version > 1)
         support::endian::write(BOS, Descriptor.Flags, llvm::endianness::little);
-    } break;
+      break;
+    }
     }
   }
   assert(Storage.size() == getSize());

llvm/lib/ObjectYAML/DXContainerEmitter.cpp

@@ -274,8 +274,8 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
       RS.StaticSamplersOffset = P.RootSignature->StaticSamplersOffset;
 
       for (const auto &Param : P.RootSignature->Parameters) {
-        auto Header = dxbc::RootParameterHeader{Param.Type, Param.Visibility,
-                                                Param.Offset};
+        dxbc::RootParameterHeader Header{Param.Type, Param.Visibility,
+                                         Param.Offset};
 
         switch (Param.Type) {
         case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit):
@@ -288,24 +288,18 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
         case llvm::to_underlying(dxbc::RootParameterType::SRV):
         case llvm::to_underlying(dxbc::RootParameterType::UAV):
         case llvm::to_underlying(dxbc::RootParameterType::CBV):
-          if (RS.Version == 1) {
-            dxbc::RTS0::v1::RootDescriptor Descriptor;
-            Descriptor.RegisterSpace = Param.Descriptor.RegisterSpace;
-            Descriptor.ShaderRegister = Param.Descriptor.ShaderRegister;
-            RS.ParametersContainer.addParameter(Header, Descriptor);
-          } else {
-            dxbc::RTS0::v2::RootDescriptor Descriptor;
-            Descriptor.RegisterSpace = Param.Descriptor.RegisterSpace;
-            Descriptor.ShaderRegister = Param.Descriptor.ShaderRegister;
+          dxbc::RTS0::v2::RootDescriptor Descriptor;
+          Descriptor.RegisterSpace = Param.Descriptor.RegisterSpace;
+          Descriptor.ShaderRegister = Param.Descriptor.ShaderRegister;
+          if (RS.Version > 1)
             Descriptor.Flags = Param.Descriptor.getEncodedFlags();
-            RS.ParametersContainer.addParameter(Header, Descriptor);
-          }
+          RS.ParametersContainer.addParameter(Header, Descriptor);
           break;
         default:
           // Handling invalid parameter type edge case. We intentionally let
           // obj2yaml/yaml2obj parse and emit invalid dxcontainer data, in order
           // for that to be used as a testing tool more effectively.
-          RS.ParametersContainer.addInfo(Header, -1);
+          RS.ParametersContainer.addInvalidParameter(Header);
         }
       }
 

llvm/lib/Target/DirectX/DXILRootSignature.cpp

@@ -76,8 +76,8 @@ static bool parseRootConstants(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD,
     return reportError(Ctx, "Invalid format for RootConstants Element");
 
   dxbc::RootParameterHeader Header;
-  // this will be properly calculated when writing it.
-  Header.ParameterOffset = 0;
+  // The parameter offset doesn't matter here - we recalculate it during
+  // serialization  Header.ParameterOffset = 0;
   Header.ParameterType =
       llvm::to_underlying(dxbc::RootParameterType::Constants32Bit);