[Mesa-dev] [RFC PATCH 19/26] glsl: reject bindless samplers/images frag inputs without 'flat'

[Samuel Pitoiset]

On 04/12/2017 02:55 AM, Timothy Arceri wrote:
> On 12/04/17 02:48, Samuel Pitoiset wrote:
>> The ARB_bindless_texture spec says:
>>
>>    "Modify Section 4.3.4, Inputs, p. 34"
>>
>>    "(modify last paragraph, p. 35, allowing samplers and images as
>>     fragment shader inputs) ... Fragment inputs can only be signed
>>     and unsigned integers and integer vectors, floating point scalars,
>>     floating-point vectors, matrices, sampler and image types, or
>>     arrays or structures of these.  Fragment shader inputs that are
>>     signed or unsigned integers, integer vectors, or any
>>     double-precision floating- point type, or any sampler or image
>>     type must be qualified with the interpolation qualifier "flat"."
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>  src/compiler/glsl/ast_to_hir.cpp | 26 ++++++++++++++++++++++++++
>>  1 file changed, 26 insertions(+)
>>
>> diff --git a/src/compiler/glsl/ast_to_hir.cpp 
>> b/src/compiler/glsl/ast_to_hir.cpp
>> index 9b8dfbcf47..d2ffa2ec69 100644
>> --- a/src/compiler/glsl/ast_to_hir.cpp
>> +++ b/src/compiler/glsl/ast_to_hir.cpp
>> @@ -3168,6 +3168,32 @@ validate_interpolation_qualifier(struct 
>> _mesa_glsl_parse_state *state,
>>        _mesa_glsl_error(loc, state, "if a fragment input is (or 
>> contains) "
>>                         "a double, then it must be qualified with 
>> 'flat'");
>>     }
>> +
>> +   /* Bindless sampler/image fragment inputs must be qualified with 
>> 'flat'.
>> +    *
>> +    * The ARB_bindless_texture spec says:
>> +    *
>> +    *    "Modify Section 4.3.4, Inputs, p. 34"
>> +    *
>> +    *    "(modify last paragraph, p. 35, allowing samplers and images as
>> +    *     fragment shader inputs) ... Fragment inputs can only be 
>> signed and
>> +    *     unsigned integers and integer vectors, floating point scalars,
>> +    *     floating-point vectors, matrices, sampler and image types, 
>> or arrays
>> +    *     or structures of these.  Fragment shader inputs that are 
>> signed or
>> +    *     unsigned integers, integer vectors, or any double-precision 
>> floating-
>> +    *     point type, or any sampler or image type must be qualified 
>> with the
>> +    *     interpolation qualifier "flat"."
>> +    */
>> +   if (state->has_bindless()
>> +       && (var_type->contains_bindless_sampler() ||
>> +           var_type->contains_bindless_image())
>> +       && interpolation != INTERP_MODE_FLAT
>> +       && state->stage == MESA_SHADER_FRAGMENT
>> +       && mode == ir_var_shader_in) {
>> +      _mesa_glsl_error(loc, state, "if a fragment input is (or 
>> contains) "
>> +                       "a bindless sampler (or image), then it must be "
>> +                       "qualified with 'flat'");
>> +   }
>>  }
>>
>>  static glsl_interp_mode
>>
> 
> 
> I think I'd like to see a patch that breaks this and the two checks 
> above into a helper functions the looks something like this:
> 
> 
> static void
> validate_fragment_flat_interpolation_input()
> {
>     if (state->stage != MESA_SHADER_FRAGMENT ||
>         interpolation == INTERP_MODE_FLAT ||
>         mode != ir_var_shader_in)
>        return;
> 
>     ... the other two checks here ...
> 
>     /* Bindless sampler/image fragment inputs must be qualified with 
> 'flat'.
>      *
>      * The ARB_bindless_texture spec says:
>      *
>      *    "Modify Section 4.3.4, Inputs, p. 34"
>      *
>      *    "(modify last paragraph, p. 35, allowing samplers and images as
>      *     fragment shader inputs) ... Fragment inputs can only be 
> signed and
>      *     unsigned integers and integer vectors, floating point scalars,
>      *     floating-point vectors, matrices, sampler and image types, or 
> arrays
>      *     or structures of these.  Fragment shader inputs that are 
> signed or
>      *     unsigned integers, integer vectors, or any double-precision 
> floating-
>      *     point type, or any sampler or image type must be qualified 
> with the
>      *     interpolation qualifier "flat"."
>      */
>     if (state->has_bindless()
>         && (var_type->contains_bindless_sampler() ||
>             var_type->contains_bindless_image())) {
>        _mesa_glsl_error(loc, state, "if a fragment input is (or contains) "
>                         "a bindless sampler (or image), then it must be "
>                         "qualified with 'flat'");
> 
> 
> }

Looks good to me.