Merge pull request #1592 from KhronosGroup/subgroupBroadcast-const-id

GLSL: Fix #1591: Require the id in subgroupBroadcast to be constant.
KhronosGroup · Nov 25, 2018 · 0e6c82c · 0e6c82c
2 parents d2a7b07 + f2675d4
commit 0e6c82c
Show file tree

Hide file tree

Showing 6 changed files with 403 additions and 373 deletions.
diff --git a/Test/baseResults/spv.subgroupBallot.comp.out b/Test/baseResults/spv.subgroupBallot.comp.out
diff --git a/Test/baseResults/spv.subgroupBallotNeg.comp.out b/Test/baseResults/spv.subgroupBallotNeg.comp.out
@@ -0,0 +1,6 @@
+spv.subgroupBallotNeg.comp
+ERROR: 0:32: 'id' : argument must be compile-time constant 
+ERROR: 1 compilation errors.  No code generated.
+
+
+SPIR-V is not generated for failed compile or link
diff --git a/Test/spv.subgroupBallot.comp b/Test/spv.subgroupBallot.comp
@@ -31,30 +31,30 @@ void main()
 
     if ((relMask == result) && subgroupInverseBallot(data[0].u4))
     {
-        data[invocation].f4.x   = subgroupBroadcast(data[0].f4.x,    invocation);
-        data[invocation].f4.xy  = subgroupBroadcast(data[1].f4.xy,   invocation);
-        data[invocation].f4.xyz = subgroupBroadcast(data[2].f4.xyz,  invocation);
-        data[invocation].f4     = subgroupBroadcast(data[3].f4,      invocation);
-
-        data[invocation].i4.x   = subgroupBroadcast(data[0].i4.x,    invocation);
-        data[invocation].i4.xy  = subgroupBroadcast(data[1].i4.xy,   invocation);
-        data[invocation].i4.xyz = subgroupBroadcast(data[2].i4.xyz,  invocation);
-        data[invocation].i4     = subgroupBroadcast(data[3].i4,      invocation);
-
-        data[invocation].u4.x   = subgroupBroadcast(data[0].u4.x,    invocation);
-        data[invocation].u4.xy  = subgroupBroadcast(data[1].u4.xy,   invocation);
-        data[invocation].u4.xyz = subgroupBroadcast(data[2].u4.xyz,  invocation);
-        data[invocation].u4     = subgroupBroadcast(data[3].u4,      invocation);
-
-        data[invocation].d4.x   = subgroupBroadcast(data[0].d4.x,    invocation);
-        data[invocation].d4.xy  = subgroupBroadcast(data[1].d4.xy,   invocation);
-        data[invocation].d4.xyz = subgroupBroadcast(data[2].d4.xyz,  invocation);
-        data[invocation].d4     = subgroupBroadcast(data[3].d4,      invocation);
-
-        data[invocation].i4.x   = int(subgroupBroadcast(data[0].i4.x < 0,            invocation));
-        data[invocation].i4.xy  = ivec2(subgroupBroadcast(lessThan(data[1].i4.xy, ivec2(0)), invocation));
-        data[invocation].i4.xyz = ivec3(subgroupBroadcast(lessThan(data[1].i4.xyz, ivec3(0)), invocation));
-        data[invocation].i4     = ivec4(subgroupBroadcast(lessThan(data[1].i4, ivec4(0)), invocation));
+        data[invocation].f4.x   = subgroupBroadcast(data[0].f4.x,    3);
+        data[invocation].f4.xy  = subgroupBroadcast(data[1].f4.xy,   3);
+        data[invocation].f4.xyz = subgroupBroadcast(data[2].f4.xyz,  3);
+        data[invocation].f4     = subgroupBroadcast(data[3].f4,      3);
+
+        data[invocation].i4.x   = subgroupBroadcast(data[0].i4.x,    2);
+        data[invocation].i4.xy  = subgroupBroadcast(data[1].i4.xy,   2);
+        data[invocation].i4.xyz = subgroupBroadcast(data[2].i4.xyz,  2);
+        data[invocation].i4     = subgroupBroadcast(data[3].i4,      2);
+
+        data[invocation].u4.x   = subgroupBroadcast(data[0].u4.x,    1);
+        data[invocation].u4.xy  = subgroupBroadcast(data[1].u4.xy,   1);
+        data[invocation].u4.xyz = subgroupBroadcast(data[2].u4.xyz,  1);
+        data[invocation].u4     = subgroupBroadcast(data[3].u4,      1);
+
+        data[invocation].d4.x   = subgroupBroadcast(data[0].d4.x,    0);
+        data[invocation].d4.xy  = subgroupBroadcast(data[1].d4.xy,   0);
+        data[invocation].d4.xyz = subgroupBroadcast(data[2].d4.xyz,  0);
+        data[invocation].d4     = subgroupBroadcast(data[3].d4,      0);
+
+        data[invocation].i4.x   = int(subgroupBroadcast(data[0].i4.x < 0,            1));
+        data[invocation].i4.xy  = ivec2(subgroupBroadcast(lessThan(data[1].i4.xy, ivec2(0)), 1));
+        data[invocation].i4.xyz = ivec3(subgroupBroadcast(lessThan(data[1].i4.xyz, ivec3(0)), 1));
+        data[invocation].i4     = ivec4(subgroupBroadcast(lessThan(data[1].i4, ivec4(0)), 1));
     }
     else
     {

diff --git a/Test/spv.subgroupBallotNeg.comp b/Test/spv.subgroupBallotNeg.comp
@@ -0,0 +1,33 @@
+#version 450
+
+#extension GL_KHR_shader_subgroup_ballot: enable
+
+layout (local_size_x = 8, local_size_y = 8, local_size_z = 1) in;
+
+layout(binding = 0) buffer Buffers
+{
+    vec4  f4;
+    ivec4 i4;
+    uvec4 u4;
+    dvec4 d4;
+} data[4];
+
+void main()
+{
+    uint invocation = (gl_SubgroupInvocationID + gl_SubgroupSize) % 4;
+
+    uvec4 relMask = gl_SubgroupEqMask +
+                       gl_SubgroupGeMask +
+                       gl_SubgroupGtMask +
+                       gl_SubgroupLeMask +
+                       gl_SubgroupLtMask;
+
+    uvec4 result = subgroupBallot(true);
+
+    data[invocation].u4.x = subgroupBallotBitCount(result);
+    data[invocation].u4.y = subgroupBallotBitExtract(result, 0) ? 1 : 0;
+    data[invocation].u4.z = subgroupBallotInclusiveBitCount(result) + subgroupBallotExclusiveBitCount(result);
+    data[invocation].u4.w = subgroupBallotFindLSB(result) + subgroupBallotFindMSB(result);
+
+    data[invocation].f4.x   = subgroupBroadcast(data[0].f4.x,    invocation);  // ERROR: not constant
+}
diff --git a/glslang/MachineIndependent/ParseHelper.cpp b/glslang/MachineIndependent/ParseHelper.cpp
@@ -1998,6 +1998,10 @@ void TParseContext::builtInOpCheck(const TSourceLoc& loc, const TFunction& fnCan
     case EOpSubgroupClusteredAnd:
     case EOpSubgroupClusteredOr:
     case EOpSubgroupClusteredXor:
+        // The <clusterSize> as used in the subgroupClustered<op>() operations must be:
+        // - An integral constant expression.
+        // - At least 1.
+        // - A power of 2.
         if ((*argp)[1]->getAsConstantUnion() == nullptr)
             error(loc, "argument must be compile-time constant", "cluster size", "");
         else {
@@ -2009,6 +2013,12 @@ void TParseContext::builtInOpCheck(const TSourceLoc& loc, const TFunction& fnCan
         }
         break;
 
+    case EOpSubgroupBroadcast:
+        // <id> must be an integral constant expression.
+        if ((*argp)[1]->getAsConstantUnion() == nullptr)
+            error(loc, "argument must be compile-time constant", "id", "");
+        break;
+
     case EOpBarrier:
     case EOpMemoryBarrier:
         if (argp->size() > 0) {

diff --git a/gtests/Spv.FromFile.cpp b/gtests/Spv.FromFile.cpp
@@ -388,6 +388,7 @@ INSTANTIATE_TEST_CASE_P(
         "spv.subgroupArithmetic.comp",
         "spv.subgroupBasic.comp",
         "spv.subgroupBallot.comp",
+        "spv.subgroupBallotNeg.comp",
         "spv.subgroupClustered.comp",
         "spv.subgroupClusteredNeg.comp",
         "spv.subgroupPartitioned.comp",