address coments

Author: joaosaffran

llvm/lib/Target/DirectX/DXILRootSignature.cpp

@@ -323,7 +323,7 @@ static bool verifyRegisterValue(uint32_t RegisterValue) {
 // This Range is reserverved, therefore invalid, according to the spec
 // https://github.com/llvm/wg-hlsl/blob/main/proposals/0002-root-signature-in-clang.md#all-the-values-should-be-legal
 static bool verifyRegisterSpace(uint32_t RegisterSpace) {
-  return !(RegisterSpace >= 0xFFFFFFF0 && RegisterSpace < 0xFFFFFFFF);
+  return !(RegisterSpace >= 0xFFFFFFF0 && RegisterSpace <= 0xFFFFFFFF);
 }
 
 static bool verifyDescriptorFlag(uint32_t Flags) { return (Flags & ~0xE) == 0; }
@@ -340,63 +340,61 @@ static bool verifyRangeType(uint32_t Type) {
   return false;
 }
 
+template <typename... FlagTypes>
+static bool isFlagSet(uint32_t Flags, FlagTypes... FlagsToCheck) {
+  return ((Flags & llvm::to_underlying(FlagsToCheck)) | ...) == Flags;
+}
+
 static bool verifyDescriptorRangeFlag(uint32_t Version, uint32_t Type,
                                       uint32_t Flags) {
-  if (Version == 1 &&
-      Type == llvm::to_underlying(dxbc::DescriptorRangeType::Sampler))
-    return Flags == 0;
-
-  if (Version == 2 &&
-      Type == llvm::to_underlying(dxbc::DescriptorRangeType::Sampler)) {
-    switch (Flags) {
-    case 0:
-    case llvm::to_underlying(dxbc::DescriptorRangeFlag::DATA_VOLATILE):
-    case llvm::to_underlying(
-        dxbc::DescriptorRangeFlag::
-            DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS):
-      return true;
-    }
-    return false;
-  }
+  bool IsSampler =
+      (Type == llvm::to_underlying(dxbc::DescriptorRangeType::Sampler));
 
-  if (Version == 1 &&
-      Type != llvm::to_underlying(dxbc::DescriptorRangeType::Sampler))
+  if (Version == 1) {
+    if (IsSampler) {
+      return Flags == 0;
+    }
     return Flags ==
            llvm::to_underlying(dxbc::DescriptorRangeFlag::DESCRIPTORS_VOLATILE);
+  }
 
-  if (Version == 2 &&
-      Type != llvm::to_underlying(dxbc::DescriptorRangeType::Sampler)) {
-    switch (Flags) {
-    case 0:
-    case llvm::to_underlying(dxbc::DescriptorRangeFlag::DESCRIPTORS_VOLATILE):
-    case llvm::to_underlying(dxbc::DescriptorRangeFlag::DATA_VOLATILE):
-    case llvm::to_underlying(dxbc::DescriptorRangeFlag::DATA_STATIC):
-    case llvm::to_underlying(
-        dxbc::DescriptorRangeFlag::DATA_STATIC_WHILE_SET_AT_EXECUTE):
-    case llvm::to_underlying(
-        dxbc::DescriptorRangeFlag::
-            DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS):
-    case llvm::to_underlying(dxbc::DescriptorRangeFlag::DESCRIPTORS_VOLATILE) |
-        llvm::to_underlying(dxbc::DescriptorRangeFlag::DATA_VOLATILE):
-    case llvm::to_underlying(dxbc::DescriptorRangeFlag::DESCRIPTORS_VOLATILE) |
-        llvm::to_underlying(
-            dxbc::DescriptorRangeFlag::DATA_STATIC_WHILE_SET_AT_EXECUTE):
-    case llvm::to_underlying(
-        dxbc::DescriptorRangeFlag::
-            DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS) |
-        llvm::to_underlying(dxbc::DescriptorRangeFlag::DATA_VOLATILE):
-    case llvm::to_underlying(
-        dxbc::DescriptorRangeFlag::
-            DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS) |
-        llvm::to_underlying(dxbc::DescriptorRangeFlag::DATA_STATIC):
-    case llvm::to_underlying(
-        dxbc::DescriptorRangeFlag::
-            DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS) |
-        llvm::to_underlying(
-            dxbc::DescriptorRangeFlag::DATA_STATIC_WHILE_SET_AT_EXECUTE):
-      return true;
+  if (Version == 2) {
+    if (IsSampler) {
+      return Flags == 0 ||
+             isFlagSet(Flags, dxbc::DescriptorRangeFlag::DATA_VOLATILE) ||
+             isFlagSet(Flags,
+                       dxbc::DescriptorRangeFlag::
+                           DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS);
     }
-    return false;
+    // Valid flag combinations for non-sampler Version 2
+    return Flags == 0 ||
+           isFlagSet(Flags, dxbc::DescriptorRangeFlag::DESCRIPTORS_VOLATILE) ||
+           isFlagSet(Flags, dxbc::DescriptorRangeFlag::DATA_VOLATILE) ||
+           isFlagSet(Flags, dxbc::DescriptorRangeFlag::DATA_STATIC) ||
+           isFlagSet(
+               Flags,
+               dxbc::DescriptorRangeFlag::DATA_STATIC_WHILE_SET_AT_EXECUTE) ||
+           isFlagSet(Flags,
+                     dxbc::DescriptorRangeFlag::
+                         DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS) ||
+           isFlagSet(Flags, dxbc::DescriptorRangeFlag::DESCRIPTORS_VOLATILE,
+                     dxbc::DescriptorRangeFlag::DATA_VOLATILE) ||
+           isFlagSet(
+               Flags, dxbc::DescriptorRangeFlag::DESCRIPTORS_VOLATILE,
+               dxbc::DescriptorRangeFlag::DATA_STATIC_WHILE_SET_AT_EXECUTE) ||
+           isFlagSet(Flags,
+                     dxbc::DescriptorRangeFlag::
+                         DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS,
+                     dxbc::DescriptorRangeFlag::DATA_VOLATILE) ||
+           isFlagSet(Flags,
+                     dxbc::DescriptorRangeFlag::
+                         DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS,
+                     dxbc::DescriptorRangeFlag::DATA_STATIC) ||
+           isFlagSet(
+               Flags,
+               dxbc::DescriptorRangeFlag::
+                   DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS,
+               dxbc::DescriptorRangeFlag::DATA_STATIC_WHILE_SET_AT_EXECUTE);
   }
   return false;
 }

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

@@ -16,5 +16,5 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 !2 = !{ ptr @main, !3 } ; function, root signature
 !3 = !{ !5 } ; list of root signature elements
 !5 = !{ !"DescriptorTable", i32 0, !6, !7 }
-!6 = !{ !"SRV", i32 0, i32 0, i32 -1, i32 -1, i32 22 }
+!6 = !{ !"SRV", i32 0, i32 1, i32 0, i32 -1, i32 22 }
 !7 = !{ !"UAV", i32 5, i32 1, i32 10, i32 5, i32 2 }