Repository: KhronosGroup/SPIRV-Tools (language: python)

Issue:
Remove some unused macros and functions.



Discussion / hints:
+2

In as 6f13c7322960f9c41d601421d2637c957541fd03.


Fix patch:
diff --git a/source/val/validate_memory.cpp b/source/val/validate_memory.cpp
index 425a8d3d22..8ff32c8f7c 100644
--- a/source/val/validate_memory.cpp
+++ b/source/val/validate_memory.cpp
@@ -438,11 +438,11 @@ spv_result_t ValidateVariable(ValidationState_t& _, const Instruction* inst) {
       storage_class != SpvStorageClassCrossWorkgroup &&
       storage_class != SpvStorageClassPrivate &&
       storage_class != SpvStorageClassFunction &&
-      storage_class != SpvStorageClassRayPayloadNV &&
-      storage_class != SpvStorageClassIncomingRayPayloadNV &&
-      storage_class != SpvStorageClassHitAttributeNV &&
-      storage_class != SpvStorageClassCallableDataNV &&
-      storage_class != SpvStorageClassIncomingCallableDataNV) {
+      storage_class != SpvStorageClassRayPayloadKHR &&
+      storage_class != SpvStorageClassIncomingRayPayloadKHR &&
+      storage_class != SpvStorageClassHitAttributeKHR &&
+      storage_class != SpvStorageClassCallableDataKHR &&
+      storage_class != SpvStorageClassIncomingCallableDataKHR) {
     bool storage_input_or_output = storage_class == SpvStorageClassInput ||
                                    storage_class == SpvStorageClassOutput;
     bool builtin = false;
@@ -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";
+      }
     }
   }
 


Test patch:
diff --git a/test/val/val_id_test.cpp b/test/val/val_id_test.cpp
index b7e504227a..a5f315c98b 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