Mere mention of PointSize BuiltIn does not require Shader capability

Fixes KhronosGroup#365 which was reopened for this.
dneto0 · Aug 24, 2016 · 58f77a2 · 58f77a2
1 parent 6f13c73
commit 58f77a2
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 10 deletions.
diff --git a/CHANGES b/CHANGES
@@ -10,8 +10,8 @@ v2016.3-dev 2016-08-11
  - Fixes issues:
    #288: Check def-use dominance rules for OpPhi (variable,parent) operands
    #340: Avoid race on mkdir during build
-   #365: Relax ClipDistance, CullDistance capability check in all environments
-     not just Vulkan 1.0.
+   #365: Relax PointSize, ClipDistance, CullDistance capability check in all
+     environments not just Vulkan 1.0.
 
 v2016.2 2016-08-05
  - Validator is incomplete

diff --git a/source/validate_instruction.cpp b/source/validate_instruction.cpp
@@ -80,14 +80,21 @@ spv_capability_mask_t RequiredCapabilities(const AssemblyGrammar& grammar,
   if (SPV_SUCCESS == grammar.lookupOperand(type, operand, &operand_desc)) {
     result = operand_desc->capabilities;
 
-    // Mere mention of ClipDistance and CullDistance in a Builtin decoration
+    // Mere mention of PointSize, ClipDistance, CullDistance in a Builtin decoration
     // does not require the associated capability.  The use of such a variable
     // value should trigger the capability requirement, but that's not
     // implemented yet.  This rule is independent of target environment.
     // See https://github.com/KhronosGroup/SPIRV-Tools/issues/365
     if (type == SPV_OPERAND_TYPE_BUILT_IN) {
-      result = result & (~(SPV_CAPABILITY_AS_MASK(SpvCapabilityClipDistance) |
-                           SPV_CAPABILITY_AS_MASK(SpvCapabilityCullDistance)));
+      switch (operand) {
+        case SpvBuiltInPointSize:
+        case SpvBuiltInClipDistance:
+        case SpvBuiltInCullDistance:
+          result = 0;
+          break;
+        default:
+          break;
+      }
     }
   }
 

diff --git a/test/Validate.Capability.cpp b/test/Validate.Capability.cpp
@@ -927,12 +927,12 @@ INSTANTIATE_TEST_CASE_P(BuiltIn, ValidateCapability,
 make_pair(string(kOpenCLMemoryModel) +
           "OpDecorate %intt BuiltIn Position\n"
           "%intt = OpTypeInt 32 1\n", ShaderDependencies()),
-make_pair(string(kOpenCLMemoryModel) +
-          "OpDecorate %intt BuiltIn PointSize\n"
-          "%intt = OpTypeInt 32 1\n", ShaderDependencies()),
-// Just mentioning ClipDistance or CullDistance as a BuiltIn does
+// Just mentioning PointSize, ClipDistance, or CullDistance as a BuiltIn does
 // not trigger the requirement for the associated capability.
 // See https://github.com/KhronosGroup/SPIRV-Tools/issues/365
+make_pair(string(kOpenCLMemoryModel) +
+          "OpDecorate %intt BuiltIn PointSize\n"
+          "%intt = OpTypeInt 32 1\n", AllCapabilities()),
 make_pair(string(kOpenCLMemoryModel) +
           "OpDecorate %intt BuiltIn ClipDistance\n"
           "%intt = OpTypeInt 32 1\n", AllCapabilities()),
@@ -1058,7 +1058,7 @@ make_pair(string(kOpenCLMemoryModel) +
           "%intt = OpTypeInt 32 1\n", ShaderDependencies())
 )),);
 
-// Ensure that mere mention of ClipDistance and CullDistance as
+// Ensure that mere mention of PointSize, ClipDistance, or CullDistance as
 // BuiltIns does not trigger the requirement for the associated
 // capability.
 // See https://github.com/KhronosGroup/SPIRV-Tools/issues/365
@@ -1067,6 +1067,9 @@ INSTANTIATE_TEST_CASE_P(BuiltIn, ValidateCapabilityVulkan10,
                             // Vulkan 1.0 is based on SPIR-V 1.0
                             ValuesIn(AllV10Capabilities()),
                             Values(
+make_pair(string(kGLSL450MemoryModel) +
+          "OpDecorate %intt BuiltIn PointSize\n"
+          "%intt = OpTypeInt 32 1\n", AllV10Capabilities()),
 make_pair(string(kGLSL450MemoryModel) +
           "OpDecorate %intt BuiltIn ClipDistance\n"
           "%intt = OpTypeInt 32 1\n", AllV10Capabilities()),
@@ -1080,6 +1083,9 @@ INSTANTIATE_TEST_CASE_P(BuiltIn, ValidateCapabilityOpenGL40,
                             // OpenGL 4.0 is based on SPIR-V 1.0
                             ValuesIn(AllV10Capabilities()),
                             Values(
+make_pair(string(kGLSL450MemoryModel) +
+          "OpDecorate %intt BuiltIn PointSize\n"
+          "%intt = OpTypeInt 32 1\n", AllV10Capabilities()),
 make_pair(string(kGLSL450MemoryModel) +
           "OpDecorate %intt BuiltIn ClipDistance\n"
           "%intt = OpTypeInt 32 1\n", AllV10Capabilities()),