refactoring suggested by inbelic

Author: joaosaffran

llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h

@@ -17,6 +17,7 @@
 #include "llvm/ADT/IntervalMap.h"
 #include "llvm/Frontend/HLSL/HLSLRootSignature.h"
 #include "llvm/Support/Compiler.h"
+#include <cstdint>
 
 namespace llvm {
 namespace hlsl {
@@ -40,9 +41,11 @@ LLVM_ABI bool verifyMaxAnisotropy(uint32_t MaxAnisotropy);
 LLVM_ABI bool verifyComparisonFunc(uint32_t ComparisonFunc);
 LLVM_ABI bool verifyBorderColor(uint32_t BorderColor);
 LLVM_ABI bool verifyLOD(float LOD);
-LLVM_ABI bool verifyOffsetOverflowing(
-    uint64_t &AppendingRegister, uint32_t OffsetInDescriptorsFromTableStart,
-    uint32_t BaseRegister, uint32_t Space, uint32_t NumDescriptors);
+LLVM_ABI bool verifyRegisterOverflow(uint64_t Register,
+                                     uint32_t NumDescriptors);
+LLVM_ABI uint64_t updateAppendingRegister(uint64_t Register,
+                                          uint32_t NumDescriptors);
+LLVM_ABI bool verifyOffsetOverflow(uint32_t Offset, uint64_t Register);
 
 } // namespace rootsig
 } // namespace hlsl

llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp

@@ -546,12 +546,22 @@ Error validateDescriptorTableRegisterOverflow(mcdxbc::DescriptorTable Table,
   for (const dxbc::RTS0::v2::DescriptorRange &Range : Table.Ranges) {
     dxbc::DescriptorRangeType RangeType =
         static_cast<dxbc::DescriptorRangeType>(Range.RangeType);
-    if (verifyOffsetOverflowing(AppendingRegister,
-                                Range.OffsetInDescriptorsFromTableStart,
-                                Range.BaseShaderRegister, Range.RegisterSpace,
-                                Range.NumDescriptors))
+
+    if (verifyOffsetOverflow(Range.OffsetInDescriptorsFromTableStart,
+                             AppendingRegister))
+      return make_error<TableRegisterOverflowError>(
+          RangeType, Range.BaseShaderRegister, Range.RegisterSpace);
+
+    if (verifyRegisterOverflow(Range.BaseShaderRegister, Range.NumDescriptors))
       return make_error<TableRegisterOverflowError>(
           RangeType, Range.BaseShaderRegister, Range.RegisterSpace);
+
+    if (verifyRegisterOverflow(AppendingRegister, Range.NumDescriptors))
+      return make_error<TableRegisterOverflowError>(
+          RangeType, Range.BaseShaderRegister, Range.RegisterSpace);
+
+    AppendingRegister =
+        updateAppendingRegister(AppendingRegister, Range.NumDescriptors);
   }
 
   return Error::success();

llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp

@@ -180,48 +180,33 @@ bool verifyBorderColor(uint32_t BorderColor) {
 
 bool verifyLOD(float LOD) { return !std::isnan(LOD); }
 
-/** This validation logic was extracted from the DXC codebase
- *   https://github.com/microsoft/DirectXShaderCompiler/blob/7a1b1df9b50a8350a63756720e85196e0285e664/lib/DxilRootSignature/DxilRootSignatureValidator.cpp#L205
- *
- *   It checks if the registers in a descriptor table are overflowing, meaning,
- *   they are trying to bind a register larger than MAX_UINT.
- *   This will usually happen when the descriptor table appends a resource
- *   after an unbounded range.
- **/
-bool verifyOffsetOverflowing(uint64_t &AppendingRegister,
-                             uint32_t OffsetInDescriptorsFromTableStart,
-                             uint32_t BaseRegister, uint32_t Space,
-                             uint32_t NumDescriptors) {
-  uint64_t Register = AppendingRegister;
-
-  // Checks if the current register should be appended to the previous range.
-  if (OffsetInDescriptorsFromTableStart != ~0U)
-    Register = OffsetInDescriptorsFromTableStart;
-
-  // Check for overflow in the register value.
+bool verifyOffsetOverflow(uint32_t Offset, uint64_t Register) {
+  if (Offset != ~0U)
+    Register = Offset;
+
   if (Register > ~0U)
     return true;
-  // Is the current range unbounded?
-  if (NumDescriptors == ~0U) {
-    // No ranges should be appended to an unbounded range.
-    AppendingRegister = (uint64_t)~0U + (uint64_t)1ULL;
-  } else {
-    // Is the defined range, overflowing?
-    uint64_t UpperBound =
-        (uint64_t)BaseRegister + (uint64_t)NumDescriptors - (uint64_t)1U;
-    if (UpperBound > ~0U)
-      return true;
-
-    // If we append this range, will it overflow?
-    uint64_t AppendingUpperBound =
-        (uint64_t)Register + (uint64_t)NumDescriptors - (uint64_t)1U;
-    if (AppendingUpperBound > ~0U)
-      return true;
-    AppendingRegister = Register + NumDescriptors;
-  }
   return false;
 }
 
+bool verifyRegisterOverflow(uint64_t Register, uint32_t NumDescriptors) {
+  if (NumDescriptors == ~0U)
+    return false;
+
+  uint64_t UpperBound =
+      (uint64_t)Register + (uint64_t)NumDescriptors - (uint64_t)1U;
+  if (UpperBound > ~0U)
+    return true;
+
+  return false;
+}
+
+uint64_t updateAppendingRegister(uint64_t AppendingRegisterRegister,
+                                 uint32_t NumDescriptors) {
+  if (NumDescriptors == ~0U)
+    return (uint64_t)~0U + (uint64_t)1ULL;
+  return AppendingRegisterRegister + NumDescriptors;
+}
 } // namespace rootsig
 } // namespace hlsl
 } // namespace llvm