spirv-val: Add static RayQuery/RayTracing value check

spencer-lunarg · spencer-lunarg · commit 784604330552 · 2024-02-14T15:53:07.000+09:00
diff --git a/source/val/validate.h b/source/val/validate.h
@@ -210,6 +210,11 @@ spv_result_t RayQueryPass(ValidationState_t& _, const Instruction* inst);
 /// Validates correctness of ray tracing instructions.
 spv_result_t RayTracingPass(ValidationState_t& _, const Instruction* inst);
 
+/// Validates constant values of the Ray Flags operand
+spv_result_t RayTracingRayFlagsOperandValue(ValidationState_t& _,
+                                            const Instruction* inst,
+                                            uint32_t ray_flags_value);
+
 /// Validates correctness of shader execution reorder instructions.
 spv_result_t RayReorderNVPass(ValidationState_t& _, const Instruction* inst);
 
diff --git a/source/val/validate_ray_query.cpp b/source/val/validate_ray_query.cpp
@@ -82,10 +82,19 @@ spv_result_t RayQueryPass(ValidationState_t& _, const Instruction* inst) {
                   "OpTypeAccelerationStructureKHR";
       }
 
-      const uint32_t ray_flags = _.GetOperandTypeId(inst, 2);
-      if (!_.IsIntScalarType(ray_flags) || _.GetBitWidth(ray_flags) != 32) {
+      const uint32_t ray_flags = inst->GetOperandAs<uint32_t>(2);
+      bool is_ray_flags_int32 = false;
+      bool is_ray_flags_const = false;
+      uint32_t ray_flags_value = 0;
+      std::tie(is_ray_flags_int32, is_ray_flags_const, ray_flags_value) =
+          _.EvalInt32IfConst(ray_flags);
+      if (!is_ray_flags_int32) {
         return _.diag(SPV_ERROR_INVALID_DATA, inst)
                << "Ray Flags must be a 32-bit int scalar";
+      } else if (is_ray_flags_const) {
+        if (auto error =
+                RayTracingRayFlagsOperandValue(_, inst, ray_flags_value))
+          return error;
       }
 
       const uint32_t cull_mask = _.GetOperandTypeId(inst, 3);
@@ -101,10 +110,20 @@ spv_result_t RayQueryPass(ValidationState_t& _, const Instruction* inst) {
                << "Ray Origin must be a 32-bit float 3-component vector";
       }
 
-      const uint32_t ray_tmin = _.GetOperandTypeId(inst, 5);
-      if (!_.IsFloatScalarType(ray_tmin) || _.GetBitWidth(ray_tmin) != 32) {
+      const uint32_t ray_tmin = inst->GetOperandAs<uint32_t>(5);
+      bool is_ray_tmin_float32 = false;
+      bool is_ray_tmin_const = false;
+      float ray_tmin_value = 0;
+      std::tie(is_ray_tmin_float32, is_ray_tmin_const, ray_tmin_value) =
+          _.EvalFloat32IfConst(ray_tmin);
+      if (!is_ray_tmin_float32) {
         return _.diag(SPV_ERROR_INVALID_DATA, inst)
                << "Ray TMin must be a 32-bit float scalar";
+      } else if (is_ray_tmin_const && ray_tmin_value < 0.0f) {
+        // Don't need to check TMax for being negative because it can't without
+        // being less than TMin
+        return _.diag(SPV_ERROR_INVALID_DATA, inst)
+               << "Ray Tmin is negative (" << ray_tmin_value << ")";
       }
 
       const uint32_t ray_direction = _.GetOperandTypeId(inst, 6);
@@ -115,11 +134,23 @@ spv_result_t RayQueryPass(ValidationState_t& _, const Instruction* inst) {
                << "Ray Direction must be a 32-bit float 3-component vector";
       }
 
-      const uint32_t ray_tmax = _.GetOperandTypeId(inst, 7);
-      if (!_.IsFloatScalarType(ray_tmax) || _.GetBitWidth(ray_tmax) != 32) {
+      const uint32_t ray_tmax = inst->GetOperandAs<uint32_t>(7);
+      bool is_ray_tmax_float32 = false;
+      bool is_ray_tmax_const = false;
+      float ray_tmax_value = 0;
+      std::tie(is_ray_tmax_float32, is_ray_tmax_const, ray_tmax_value) =
+          _.EvalFloat32IfConst(ray_tmax);
+      if (!is_ray_tmax_float32) {
         return _.diag(SPV_ERROR_INVALID_DATA, inst)
                << "Ray TMax must be a 32-bit float scalar";
       }
+
+      if (is_ray_tmin_const && is_ray_tmax_const &&
+          ray_tmin_value > ray_tmax_value) {
+        return _.diag(SPV_ERROR_INVALID_DATA, inst)
+               << "Ray Tmin (" << ray_tmin_value
+               << ") is larger than Ray Tmax (" << ray_tmax_value << ")";
+      }
       break;
     }
 
diff --git a/source/val/validate_ray_tracing.cpp b/source/val/validate_ray_tracing.cpp
@@ -22,6 +22,41 @@
 namespace spvtools {
 namespace val {
 
+spv_result_t RayTracingRayFlagsOperandValue(ValidationState_t& _,
+                                            const Instruction* inst,
+                                            uint32_t ray_flags_value) {
+  const auto HasMoreThenOneBitSet =
+      [ray_flags_value](const spv::RayFlagsMask ray_flag_mask) {
+        const uint32_t mask =
+            ray_flags_value & static_cast<uint32_t>(ray_flag_mask);
+        return mask != 0 && (mask & (mask - 1));
+      };
+
+  if (HasMoreThenOneBitSet(spv::RayFlagsMask::SkipAABBsKHR |
+                           spv::RayFlagsMask::SkipTrianglesKHR)) {
+    return _.diag(SPV_ERROR_INVALID_DATA, inst)
+           << "Ray Flags contains both SkipTrianglesKHR and SkipAABBsKHR";
+  }
+
+  if (HasMoreThenOneBitSet(spv::RayFlagsMask::SkipTrianglesKHR |
+                           spv::RayFlagsMask::CullFrontFacingTrianglesKHR |
+                           spv::RayFlagsMask::CullBackFacingTrianglesKHR)) {
+    return _.diag(SPV_ERROR_INVALID_DATA, inst)
+           << "Ray Flags contains more than one of SkipTrianglesKHR or "
+              "CullFrontFacingTrianglesKHR or CullBackFacingTrianglesKHR";
+  }
+
+  if (HasMoreThenOneBitSet(spv::RayFlagsMask::OpaqueKHR |
+                           spv::RayFlagsMask::NoOpaqueKHR |
+                           spv::RayFlagsMask::CullOpaqueKHR |
+                           spv::RayFlagsMask::CullNoOpaqueKHR)) {
+    return _.diag(SPV_ERROR_INVALID_DATA, inst)
+           << "Ray Flags contains more than one of OpaqueKHR or NoOpaqueKHR or "
+              "CullOpaqueKHR or CullNoOpaqueKHR";
+  }
+  return SPV_SUCCESS;
+}
+
 spv_result_t RayTracingPass(ValidationState_t& _, const Instruction* inst) {
   const spv::Op opcode = inst->opcode();
   const uint32_t result_type = inst->type_id();
@@ -51,10 +86,19 @@ spv_result_t RayTracingPass(ValidationState_t& _, const Instruction* inst) {
                   "OpTypeAccelerationStructureKHR";
       }
 
-      const uint32_t ray_flags = _.GetOperandTypeId(inst, 1);
-      if (!_.IsIntScalarType(ray_flags) || _.GetBitWidth(ray_flags) != 32) {
+      const uint32_t ray_flags = inst->GetOperandAs<uint32_t>(1);
+      bool is_ray_flags_int32 = false;
+      bool is_ray_flags_const = false;
+      uint32_t ray_flags_value = 0;
+      std::tie(is_ray_flags_int32, is_ray_flags_const, ray_flags_value) =
+          _.EvalInt32IfConst(ray_flags);
+      if (!is_ray_flags_int32) {
         return _.diag(SPV_ERROR_INVALID_DATA, inst)
                << "Ray Flags must be a 32-bit int scalar";
+      } else if (is_ray_flags_const) {
+        if (auto error =
+                RayTracingRayFlagsOperandValue(_, inst, ray_flags_value))
+          return error;
       }
 
       const uint32_t cull_mask = _.GetOperandTypeId(inst, 2);
@@ -88,10 +132,20 @@ spv_result_t RayTracingPass(ValidationState_t& _, const Instruction* inst) {
                << "Ray Origin must be a 32-bit float 3-component vector";
       }
 
-      const uint32_t ray_tmin = _.GetOperandTypeId(inst, 7);
-      if (!_.IsFloatScalarType(ray_tmin) || _.GetBitWidth(ray_tmin) != 32) {
+      const uint32_t ray_tmin = inst->GetOperandAs<uint32_t>(7);
+      bool is_ray_tmin_float32 = false;
+      bool is_ray_tmin_const = false;
+      float ray_tmin_value = 0;
+      std::tie(is_ray_tmin_float32, is_ray_tmin_const, ray_tmin_value) =
+          _.EvalFloat32IfConst(ray_tmin);
+      if (!is_ray_tmin_float32) {
         return _.diag(SPV_ERROR_INVALID_DATA, inst)
                << "Ray TMin must be a 32-bit float scalar";
+      } else if (is_ray_tmin_const && ray_tmin_value < 0.0f) {
+        // Don't need to check TMax for being negative because it can't without
+        // being less than TMin
+        return _.diag(SPV_ERROR_INVALID_DATA, inst)
+               << "Ray Tmin is negative (" << ray_tmin_value << ")";
       }
 
       const uint32_t ray_direction = _.GetOperandTypeId(inst, 8);
@@ -102,12 +156,24 @@ spv_result_t RayTracingPass(ValidationState_t& _, const Instruction* inst) {
                << "Ray Direction must be a 32-bit float 3-component vector";
       }
 
-      const uint32_t ray_tmax = _.GetOperandTypeId(inst, 9);
-      if (!_.IsFloatScalarType(ray_tmax) || _.GetBitWidth(ray_tmax) != 32) {
+      const uint32_t ray_tmax = inst->GetOperandAs<uint32_t>(9);
+      bool is_ray_tmax_float32 = false;
+      bool is_ray_tmax_const = false;
+      float ray_tmax_value = 0;
+      std::tie(is_ray_tmax_float32, is_ray_tmax_const, ray_tmax_value) =
+          _.EvalFloat32IfConst(ray_tmax);
+      if (!is_ray_tmax_float32) {
         return _.diag(SPV_ERROR_INVALID_DATA, inst)
                << "Ray TMax must be a 32-bit float scalar";
       }
 
+      if (is_ray_tmin_const && is_ray_tmax_const &&
+          ray_tmin_value > ray_tmax_value) {
+        return _.diag(SPV_ERROR_INVALID_DATA, inst)
+               << "Ray Tmin (" << ray_tmin_value
+               << ") is larger than Ray Tmax (" << ray_tmax_value << ")";
+      }
+
       const Instruction* payload = _.FindDef(inst->GetOperandAs<uint32_t>(10));
       if (payload->opcode() != spv::Op::OpVariable) {
         return _.diag(SPV_ERROR_INVALID_DATA, inst)
diff --git a/source/val/validation_state.cpp b/source/val/validation_state.cpp
@@ -15,6 +15,7 @@
 #include "source/val/validation_state.h"
 
 #include <cassert>
+#include <cstring>
 #include <stack>
 #include <utility>
 
@@ -1374,6 +1375,34 @@ std::tuple<bool, bool, uint32_t> ValidationState_t::EvalInt32IfConst(
   return std::make_tuple(true, true, inst->word(3));
 }
 
+std::tuple<bool, bool, float> ValidationState_t::EvalFloat32IfConst(
+    uint32_t id) const {
+  const Instruction* const inst = FindDef(id);
+  assert(inst);
+  const uint32_t type = inst->type_id();
+
+  if (type == 0 || !IsFloatScalarType(type) || GetBitWidth(type) != 32) {
+    return std::make_tuple(false, false, 0.0f);
+  }
+
+  // Spec constant values cannot be evaluated so don't consider constant for
+  // the purpose of this method.
+  if (!spvOpcodeIsConstant(inst->opcode()) ||
+      spvOpcodeIsSpecConstant(inst->opcode())) {
+    return std::make_tuple(true, false, 0.0f);
+  }
+
+  if (inst->opcode() == spv::Op::OpConstantNull) {
+    return std::make_tuple(true, true, 0.0f);
+  }
+
+  assert(inst->words().size() == 4);
+  uint32_t word = inst->word(3);
+  float value = 0;
+  std::memcpy(&value, &word, sizeof(float));
+  return std::make_tuple(true, true, value);
+}
+
 void ValidationState_t::ComputeFunctionToEntryPointMapping() {
   for (const uint32_t entry_point : entry_points()) {
     std::stack<uint32_t> call_stack;
diff --git a/source/val/validation_state.h b/source/val/validation_state.h
@@ -730,6 +730,9 @@ class ValidationState_t {
   // OpSpecConstant* return |is_const_int32| as false since their values cannot
   // be relied upon during validation.
   std::tuple<bool, bool, uint32_t> EvalInt32IfConst(uint32_t id) const;
+  // Tries to evaluate a 32-bit scalar float constant.
+  // Returns tuple <is_float32, is_const_float32, value>.
+  std::tuple<bool, bool, float> EvalFloat32IfConst(uint32_t id) const;
 
   // Returns the disassembly string for the given instruction.
   std::string Disassemble(const Instruction& inst) const;
diff --git a/test/val/val_ray_query_test.cpp b/test/val/val_ray_query_test.cpp
diff --git a/test/val/val_ray_tracing_test.cpp b/test/val/val_ray_tracing_test.cpp
