spirv-val: Better message for using OpTypeBool in input/output (#4901)

2 files changed, 66 insertions, 10 deletions

diff --git a/source/val/validate_memory.cpp b/source/val/validate_memory.cpp
index 5571aa64..d4fca84a 100644
--- a/source/val/validate_memory.cpp
+++ b/source/val/validate_memory.cpp
@@ -454,12 +454,23 @@ spv_result_t ValidateVariable(ValidationState_t& _, const Instruction* inst) {
         }
       }
     }
-    if (!(storage_input_or_output && builtin) &&
+    if (!builtin &&
         ContainsInvalidBool(_, value_type, storage_input_or_output)) {
-      return _.diag(SPV_ERROR_INVALID_ID, inst)
-             << "If OpTypeBool is stored in conjunction with OpVariable, it "
-             << "can only be used with non-externally visible shader Storage "
-             << "Classes: Workgroup, CrossWorkgroup, Private, and Function";
+      if (storage_input_or_output) {
+        return _.diag(SPV_ERROR_INVALID_ID, inst)
+               << "If OpTypeBool is stored in conjunction with OpVariable "
+                  "using Input or Output Storage Classes it requires a BuiltIn "
+                  "decoration";
+
+      } else {
+        return _.diag(SPV_ERROR_INVALID_ID, inst)
+               << "If OpTypeBool is stored in conjunction with OpVariable, it "
+                  "can only be used with non-externally visible shader Storage "
+                  "Classes: Workgroup, CrossWorkgroup, Private, Function, "
+                  "Input, Output, RayPayloadKHR, IncomingRayPayloadKHR, "
+                  "HitAttributeKHR, CallableDataKHR, or "
+                  "IncomingCallableDataKHR";
+      }
     }
   }
 
diff --git a/test/val/val_id_test.cpp b/test/val/val_id_test.cpp
index b7e50422..a5f315c9 100644
--- a/test/val/val_id_test.cpp
+++ b/test/val/val_id_test.cpp
@@ -2187,11 +2187,33 @@ OpFunctionEnd
 )";
   CompileSuccessfully(spirv.c_str());
   EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
-  EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("If OpTypeBool is stored in conjunction with OpVariable"
-                        ", it can only be used with non-externally visible "
-                        "shader Storage Classes: Workgroup, CrossWorkgroup, "
-                        "Private, and Function"));
+  EXPECT_THAT(
+      getDiagnosticString(),
+      HasSubstr(
+          "If OpTypeBool is stored in conjunction with OpVariable, it can only "
+          "be used with non-externally visible shader Storage Classes: "
+          "Workgroup, CrossWorkgroup, Private, Function, Input, Output, "
+          "RayPayloadKHR, IncomingRayPayloadKHR, HitAttributeKHR, "
+          "CallableDataKHR, or IncomingCallableDataKHR"));
+}
+
+TEST_F(ValidateIdWithMessage, OpVariableContainsBoolPrivateGood) {
+  std::string spirv = kGLSL450MemoryModel + R"(
+%bool = OpTypeBool
+%int = OpTypeInt 32 0
+%block = OpTypeStruct %bool %int
+%_ptr_Private_block = OpTypePointer Private %block
+%var = OpVariable %_ptr_Private_block Private
+%void = OpTypeVoid
+%fnty = OpTypeFunction %void
+%main = OpFunction %void None %fnty
+%entry = OpLabel
+%load = OpLoad %block %var
+OpReturn
+OpFunctionEnd
+)";
+  CompileSuccessfully(spirv.c_str());
+  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
 }
 
 TEST_F(ValidateIdWithMessage, OpVariableContainsBoolPointerGood) {
@@ -2233,6 +2255,29 @@ OpFunctionEnd
   EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
 }
 
+TEST_F(ValidateIdWithMessage, OpVariableContainsNoBuiltinBoolBad) {
+  std::string spirv = kGLSL450MemoryModel + R"(
+%bool = OpTypeBool
+%input = OpTypeStruct %bool
+%_ptr_input = OpTypePointer Input %input
+%var = OpVariable %_ptr_input Input
+%void = OpTypeVoid
+%fnty = OpTypeFunction %void
+%main = OpFunction %void None %fnty
+%entry = OpLabel
+%load = OpLoad %input %var
+OpReturn
+OpFunctionEnd
+)";
+  CompileSuccessfully(spirv.c_str());
+  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(
+      getDiagnosticString(),
+      HasSubstr(
+          "If OpTypeBool is stored in conjunction with OpVariable using Input "
+          "or Output Storage Classes it requires a BuiltIn decoration"));
+}
+
 TEST_F(ValidateIdWithMessage, OpVariableContainsRayPayloadBoolGood) {
   std::string spirv = R"(
 OpCapability RayTracingNV