address comments

Author: joaosaffran

llvm/include/llvm/BinaryFormat/DXContainerConstants.def

@@ -100,12 +100,11 @@ DESCRIPTOR_RANGE_FLAG(0x10000, DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS)
 // DESCRIPTOR_RANGE(value, name).
 #ifdef DESCRIPTOR_RANGE
 
-DESCRIPTOR_RANGE(4, ERROR)
 DESCRIPTOR_RANGE(0, SRV)
 DESCRIPTOR_RANGE(1, UAV)
 DESCRIPTOR_RANGE(2, CBV)
 DESCRIPTOR_RANGE(3, Sampler)
-DESCRIPTOR_RANGE(0, NONE)
+DESCRIPTOR_RANGE(4, NONE)
 #undef DESCRIPTOR_RANGE
 #endif // DESCRIPTOR_RANGE
 

llvm/lib/Target/DirectX/DXILRootSignature.cpp

@@ -197,7 +197,10 @@ static bool parseDescriptorRange(LLVMContext *Ctx,
           .Case("UAV", llvm::to_underlying(dxbc::DescriptorRangeType::UAV))
           .Case("Sampler",
                 llvm::to_underlying(dxbc::DescriptorRangeType::Sampler))
-          .Default(llvm::to_underlying(dxbc::DescriptorRangeType::ERROR));
+          .Default(-1u);
+
+  if (Range.RangeType == -1u)
+    return reportError(Ctx, "Invalid Descriptor Range type: " + *ElementText);
 
   if (std::optional<uint32_t> Val = extractMdIntValue(RangeDescriptorNode, 1))
     Range.NumDescriptors = *Val;
@@ -349,16 +352,19 @@ static bool verifyDescriptorRangeFlag(uint32_t Version, uint32_t Type,
   const bool IsSampler =
       (Type == llvm::to_underlying(dxbc::DescriptorRangeType::Sampler));
 
+  if (Version == 1) {
+    if (IsSampler)
+      return Flags == FlagT::NONE;
+    return Flags == FlagT::DESCRIPTORS_VOLATILE;
+  }
+
   // The data-specific flags are mutually exclusive.
   FlagT DataFlags = FlagT::DATA_VOLATILE | FlagT::DATA_STATIC |
                     FlagT::DATA_STATIC_WHILE_SET_AT_EXECUTE;
 
   if (popcount(llvm::to_underlying(Flags & DataFlags)) > 1)
     return false;
 
-  if (popcount(llvm::to_underlying(Flags & DataFlags)) == 1)
-    return true;
-
   // For volatile descriptors, DATA_STATIC is never valid.
   if ((Flags & FlagT::DESCRIPTORS_VOLATILE) == FlagT::DESCRIPTORS_VOLATILE) {
     FlagT Mask = FlagT::DESCRIPTORS_VOLATILE;
@@ -369,15 +375,21 @@ static bool verifyDescriptorRangeFlag(uint32_t Version, uint32_t Type,
     return (Flags & ~Mask) == FlagT::NONE;
   }
 
-  // For default (static) or "STATIC_KEEPING_BUFFER_BOUNDS_CHECKS" descriptors,
+  // For "STATIC_KEEPING_BUFFER_BOUNDS_CHECKS" descriptors,
   // the other data-specific flags may all be set.
-  FlagT Mask = FlagT::DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS;
-  if (!IsSampler) {
-    Mask |= FlagT::DATA_VOLATILE;
-    Mask |= FlagT::DATA_STATIC;
-    Mask |= FlagT::DATA_STATIC_WHILE_SET_AT_EXECUTE;
+  if ((Flags & FlagT::DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS) ==
+      FlagT::DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS) {
+    FlagT Mask = FlagT::DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS;
+    if (!IsSampler) {
+      Mask |= FlagT::DATA_VOLATILE;
+      Mask |= FlagT::DATA_STATIC;
+      Mask |= FlagT::DATA_STATIC_WHILE_SET_AT_EXECUTE;
+    }
+    return (Flags & ~Mask) == FlagT::NONE;
   }
-  return (Flags & ~Mask) == FlagT::NONE;
+
+  // When no descriptor flag is set, any data flag is allowed.
+  return (Flags & ~DataFlags) == FlagT::NONE;
 }
 
 static bool validate(LLVMContext *Ctx, const mcdxbc::RootSignatureDesc &RSD) {

llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-RangeType.ll

@@ -2,7 +2,7 @@
 
 target triple = "dxil-unknown-shadermodel6.0-compute"
 
-; CHECK: error: Invalid value for RangeType: 4
+; CHECK: error: Invalid Descriptor Range type: Invalid 
 ; CHECK-NOT: Root Signature Definitions
 
 define void @main() #0 {