spirv-val: Only validate local size with non-spec const

Author: spencer-lunarg

source/val/validate_builtins.cpp

@@ -3170,19 +3170,24 @@ spv_result_t BuiltInsValidator::ValidateWorkgroupSizeAtDefinition(
                 "constant. "
              << GetIdDesc(inst) << " is not a constant.";
       }
-    } else {
-        // can only validate product if static
+    } else if (inst.opcode() == spv::Op::OpConstantComposite) {
+      // can only validate product if static and not spec constant
+      if (_.FindDef(inst.word(3))->opcode() == spv::Op::OpConstant &&
+          _.FindDef(inst.word(4))->opcode() == spv::Op::OpConstant &&
+          _.FindDef(inst.word(5))->opcode() == spv::Op::OpConstant) {
         uint64_t x_size, y_size, z_size;
         // ValidateI32Vec above confirms there will be 3 words to read
         bool static_x = _.GetConstantValUint64(inst.word(3), &x_size);
         bool static_y = _.GetConstantValUint64(inst.word(4), &y_size);
         bool static_z = _.GetConstantValUint64(inst.word(5), &z_size);
         if (static_x && static_y && static_z &&
             ((x_size * y_size * z_size) == 0)) {
-        return _.diag(SPV_ERROR_INVALID_DATA, &inst)
-                << "WorkgroupSize decorations must not have a static "
-                    "product of zero (X = " << x_size << ", Y = " << y_size << ", Z = " << z_size << ").";
+          return _.diag(SPV_ERROR_INVALID_DATA, &inst)
+                 << "WorkgroupSize decorations must not have a static "
+                    "product of zero (X = "
+                 << x_size << ", Y = " << y_size << ", Z = " << z_size << ").";
         }
+      }
     }
 
   // Seed at reference checks with this built-in.

source/val/validate_mode_setting.cpp

@@ -340,18 +340,28 @@ spv_result_t ValidateEntryPoint(ValidationState_t& _, const Instruction* inst) {
   const Instruction* local_size_id_inst =
       _.GetLocalSizeIdInstruction(entry_point_id);
   if (local_size_id_inst) {
-    uint64_t x_size, y_size, z_size;
-    bool static_x =
-        _.GetConstantValUint64(local_size_id_inst->word(3), &x_size);
-    bool static_y =
-        _.GetConstantValUint64(local_size_id_inst->word(4), &y_size);
-    bool static_z =
-        _.GetConstantValUint64(local_size_id_inst->word(5), &z_size);
-    if (static_x && static_y && static_z && ((x_size * y_size * z_size) == 0)) {
-      return _.diag(SPV_ERROR_INVALID_DATA, local_size_inst)
-             << "Local Size Id execution mode must not have a product of zero "
-                "(X = "
-             << x_size << ", Y = " << y_size << ", Z = " << z_size << ").";
+    // can only validate product if static and not spec constant
+    if (_.FindDef(local_size_id_inst->word(3))->opcode() ==
+            spv::Op::OpConstant &&
+        _.FindDef(local_size_id_inst->word(4))->opcode() ==
+            spv::Op::OpConstant &&
+        _.FindDef(local_size_id_inst->word(5))->opcode() ==
+            spv::Op::OpConstant) {
+      uint64_t x_size, y_size, z_size;
+      bool static_x =
+          _.GetConstantValUint64(local_size_id_inst->word(3), &x_size);
+      bool static_y =
+          _.GetConstantValUint64(local_size_id_inst->word(4), &y_size);
+      bool static_z =
+          _.GetConstantValUint64(local_size_id_inst->word(5), &z_size);
+      if (static_x && static_y && static_z &&
+          ((x_size * y_size * z_size) == 0)) {
+        return _.diag(SPV_ERROR_INVALID_DATA, local_size_inst)
+               << "Local Size Id execution mode must not have a product of "
+                  "zero "
+                  "(X = "
+               << x_size << ", Y = " << y_size << ", Z = " << z_size << ").";
+      }
     }
   }
 

test/val/val_modes_test.cpp

@@ -123,11 +123,24 @@ OpDecorate %int3_1 BuiltIn WorkgroupSize
 )" + kVoidFunction;
 
   CompileSuccessfully(spirv);
-  EXPECT_THAT(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(
-      getDiagnosticString(),
-      HasSubstr(
-          "WorkgroupSize decorations must not have a static product of zero"));
+  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
+}
+
+TEST_F(ValidateMode, GLComputeZeroSpecCompositeWorkgroupSize) {
+  const std::string spirv = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint GLCompute %main "main"
+OpDecorate %int3_1 BuiltIn WorkgroupSize
+%int = OpTypeInt 32 0
+%int3 = OpTypeVector %int 3
+%int_0 = OpSpecConstant %int 0
+%int_1 = OpSpecConstant %int 1
+%int3_1 = OpSpecConstantComposite %int3 %int_1 %int_0 %int_0
+)" + kVoidFunction;
+
+  CompileSuccessfully(spirv);
+  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
 }
 
 TEST_F(ValidateMode, KernelZeroWorkgroupSizeConstant) {
@@ -280,11 +293,7 @@ OpExecutionModeId %main LocalSizeId %int_1 %int_0 %int_1
 
   spv_target_env env = SPV_ENV_UNIVERSAL_1_3;
   CompileSuccessfully(spirv, env);
-  EXPECT_THAT(SPV_ERROR_INVALID_DATA, ValidateInstructions(env));
-  EXPECT_THAT(
-      getDiagnosticString(),
-      HasSubstr(
-          "Local Size Id execution mode must not have a product of zero"));
+  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(env));
 }
 
 TEST_F(ValidateMode, KernelZeroLocalSizeId) {