[Mesa-dev] [PATCH 05/11] glsl: reject compute shaders with fixed and variable local size

[Samuel Pitoiset]

On 09/09/2016 08:46 PM, Ian Romanick wrote:
> On 09/08/2016 01:31 PM, Samuel Pitoiset wrote:
>> The ARB_compute_variable_group_size specification explains that
>> when a compute shader includes both a fixed and a variable local
>> size, a compile-time error occurs.
>
> I probably would have squashed this in with the previous commit, but
> it's fine either way.
>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>  src/compiler/glsl/ast_to_hir.cpp | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
>> index 4fc4c5c..a53a82e 100644
>> --- a/src/compiler/glsl/ast_to_hir.cpp
>> +++ b/src/compiler/glsl/ast_to_hir.cpp
>> @@ -8013,6 +8013,20 @@ ast_cs_input_layout::hir(exec_list *instructions,
>>        }
>>     }
>>
>> +   /* From the ARB_compute_variable_group_size specification:
>
> Same comment about spec quotes.

Sure.

>
> It's hard to tell from just the patch... is this check order dependent?
> Does it correct handle all the cases:
>
> // case 1
> layout(local_size_variable, local_size_x = 32, local_size_y = 32);
>
> // case 2
> layout(local_size_x = 32, local_size_y = 32, local_size_variable);
>
> // case 3
> layout(local_size_variable);
> layout(local_size_x = 32, local_size_y = 32);
>
> // case 4
> layout(local_size_x = 32, local_size_y = 32);
> layout(local_size_variable);

Yes, it does, I have just tested locally.

>
>> +    *
>> +    *     If a compute shader including a *local_size_variable* qualifier also
>> +    *     declares a fixed local group size using the *local_size_x*,
>> +    *     *local_size_y*, or *local_size_z* qualifiers, a compile-time error
>> +    *     results
>> +    */
>> +   if (state->cs_input_local_size_variable_specified) {
>> +      _mesa_glsl_error(&loc, state,
>> +                       "compute shader can't include both a variable and a "
>> +                       "fixed local group size");
>> +      return NULL;
>> +   }
>> +
>>     state->cs_input_local_size_specified = true;
>>     for (int i = 0; i < 3; i++)
>>        state->cs_input_local_size[i] = qual_local_size[i];
>>
>