addressing pr comments

Author: joaosaffran

llvm/include/llvm/BinaryFormat/DXContainer.h

@@ -165,11 +165,11 @@ enum class RootParameterType : uint32_t {
 ArrayRef<EnumEntry<RootParameterType>> getRootParameterTypes();
 
 #define SHADER_VISIBILITY(Val, Enum) Enum = Val,
-enum class ShaderVisibilityFlag : uint32_t {
+enum class ShaderVisibility : uint32_t {
 #include "DXContainerConstants.def"
 };
 
-ArrayRef<EnumEntry<ShaderVisibilityFlag>> getShaderVisibilityFlags();
+ArrayRef<EnumEntry<ShaderVisibility>> getShaderVisibility();
 
 PartType parsePartType(StringRef S);
 
@@ -565,8 +565,6 @@ struct RootConstants {
   uint32_t RegisterSpace = 0;
   uint32_t Num32BitValues = 0;
 
-  RootConstants() = default;
-
   void swapBytes() {
     sys::swapByteOrder(ShaderRegister);
     sys::swapByteOrder(RegisterSpace);
@@ -579,16 +577,14 @@ struct RootParameter {
   union {
     dxbc::RootConstants Constants;
   };
-  dxbc::ShaderVisibilityFlag ShaderVisibility;
+  dxbc::ShaderVisibility ShaderVisibility;
 
   RootParameter() {
-    Constants = RootConstants();
     ParameterType = dxbc::RootParameterType::Empty;
-    ShaderVisibility = dxbc::ShaderVisibilityFlag::Empty;
+    ShaderVisibility = dxbc::ShaderVisibility::Empty;
   }
 
   void swapBytes() {
-
     sys::swapByteOrder(ParameterType);
     sys::swapByteOrder(ShaderVisibility);
     switch (ParameterType) {
@@ -620,14 +616,14 @@ struct RootSignatureValidations {
     return (Version == 1 || Version == 2);
   }
 
-  static bool isValidParameterType(dxbc::RootParameterType Flag) {
+  static bool isValidParameterType(dxbc::RootParameterType Type) {
     // RootParameterType::Empty is the higest value in the enum.
-    return Flag < dxbc::RootParameterType::Empty;
+    return Type < dxbc::RootParameterType::Empty;
   }
 
-  static bool isValidShaderVisibility(dxbc::ShaderVisibilityFlag Flag) {
+  static bool isValidShaderVisibility(dxbc::ShaderVisibility Visibility) {
     // ShaderVisibilityFlag::Empty is the higest value in the enum.
-    return Flag < dxbc::ShaderVisibilityFlag::Empty;
+    return Visibility < dxbc::ShaderVisibility::Empty;
   }
 };
 

llvm/include/llvm/ObjectYAML/DXContainerYAML.h

@@ -202,7 +202,7 @@ LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::D3DSystemValue)
 LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::SigComponentType)
 LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::SigMinPrecision)
 LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::RootParameterType)
-LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::ShaderVisibilityFlag)
+LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::ShaderVisibility)
 
 namespace llvm {
 

llvm/lib/BinaryFormat/DXContainer.cpp

@@ -60,14 +60,14 @@ ArrayRef<EnumEntry<SigComponentType>> dxbc::getSigComponentTypes() {
   return ArrayRef(SigComponentTypes);
 }
 
-#define SHADER_VISIBILITY(Val, Enum) {#Enum, ShaderVisibilityFlag::Enum},
+#define SHADER_VISIBILITY(Val, Enum) {#Enum, ShaderVisibility::Enum},
 
-static const EnumEntry<ShaderVisibilityFlag> ShaderVisibilityFlags[] = {
+static const EnumEntry<ShaderVisibility> ShaderVisibilityValues[] = {
 #include "llvm/BinaryFormat/DXContainerConstants.def"
 };
 
-ArrayRef<EnumEntry<ShaderVisibilityFlag>> dxbc::getShaderVisibilityFlags() {
-  return ArrayRef(ShaderVisibilityFlags);
+ArrayRef<EnumEntry<ShaderVisibility>> dxbc::getShaderVisibility() {
+  return ArrayRef(ShaderVisibilityValues);
 }
 
 #define ROOT_PARAMETER(Val, Enum) {#Enum, RootParameterType::Enum},

llvm/lib/Object/DXContainer.cpp

@@ -293,28 +293,23 @@ Error DirectX::RootSignature::parse(StringRef Data) {
   for (uint32_t It = 0; It < NumParameters; It++) {
     dxbc::RootParameter NewParam;
 
-    dxbc::RootParameterType PTValue =
+    NewParam.ParameterType =
         support::endian::read<dxbc::RootParameterType,
                               llvm::endianness::little>(Current);
-    if (!dxbc::RootSignatureValidations::isValidParameterType(PTValue))
+    if (!dxbc::RootSignatureValidations::isValidParameterType(NewParam.ParameterType))
       return validationFailed("unsupported parameter type value read: " +
-                              llvm::Twine((uint32_t)PTValue));
+                              llvm::Twine((uint32_t)NewParam.ParameterType));
 
-    NewParam.ParameterType =
-        support::endian::read<dxbc::RootParameterType,
-                              llvm::endianness::little>(Current);
     Current += sizeof(dxbc::RootParameterType);
 
-    dxbc::ShaderVisibilityFlag SVValue =
-        support::endian::read<dxbc::ShaderVisibilityFlag,
+    NewParam.ShaderVisibility =
+        support::endian::read<dxbc::ShaderVisibility,
                               llvm::endianness::little>(Current);
-
-    if (!dxbc::RootSignatureValidations::isValidShaderVisibility(SVValue))
+    if (!dxbc::RootSignatureValidations::isValidShaderVisibility(NewParam.ShaderVisibility))
       return validationFailed("unsupported shader visility flag value read: " +
-                              llvm::Twine((uint32_t)SVValue));
+                              llvm::Twine((uint32_t)NewParam.ShaderVisibility));
 
-    NewParam.ShaderVisibility = SVValue;
-    Current += sizeof(dxbc::ShaderVisibilityFlag);
+    Current += sizeof(dxbc::ShaderVisibility);
 
     uint32_t Offset =
         support::endian::read<uint32_t, llvm::endianness::little>(Current);
@@ -327,6 +322,7 @@ Error DirectX::RootSignature::parse(StringRef Data) {
         return Err;
     } break;
     case dxbc::RootParameterType::Empty:
+      // unreachable  because it was validated and assigned before this point.
       llvm_unreachable("Invalid value for RootParameterType");
     }
 

llvm/lib/ObjectYAML/DXContainerYAML.cpp

@@ -348,9 +348,9 @@ void ScalarEnumerationTraits<dxbc::RootParameterType>::enumeration(
     IO.enumCase(Value, E.Name.str().c_str(), E.Value);
 }
 
-void ScalarEnumerationTraits<dxbc::ShaderVisibilityFlag>::enumeration(
-    IO &IO, dxbc::ShaderVisibilityFlag &Value) {
-  for (const auto &E : dxbc::getShaderVisibilityFlags())
+void ScalarEnumerationTraits<dxbc::ShaderVisibility>::enumeration(
+    IO &IO, dxbc::ShaderVisibility &Value) {
+  for (const auto &E : dxbc::getShaderVisibility())
     IO.enumCase(Value, E.Name.str().c_str(), E.Value);
 }