[Mesa-dev] [PATCH 3/7] glsl: Forbid opaque variables as operands of the ternary operator.

[Francisco Jerez]

Ian Romanick <idr at freedesktop.org> writes:

> On 01/31/2015 09:54 PM, Francisco Jerez wrote:
>> ---
>>  src/glsl/ast_to_hir.cpp | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>> 
>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>> index 1ba29f7..783384e 100644
>> --- a/src/glsl/ast_to_hir.cpp
>> +++ b/src/glsl/ast_to_hir.cpp
>> @@ -1596,6 +1596,18 @@ ast_expression::do_hir(exec_list *instructions,
>>           error_emitted = true;
>>        }
>>  
>> +      /* From section 4.1.7 of the GLSL 4.50 spec (Opaque Types):
>> +       *
>> +       *  "Except for array indexing, structure member selection, and
>> +       *   parentheses, opaque variables are not allowed to be operands in
>> +       *   expressions; such use results in a compile-time error."
>> +       */
>> +      if (type->contains_opaque()) {
>> +         _mesa_glsl_error(&loc, state, "opaque variables cannot be operands "
>> +                          "of the ?: operator");
>> +         error_emitted = true;
>> +      }
>> +
>
> This is what the spec says, but I'm suspicious that other vendors may
> also allow it.  Can we poke at AMD and NVIDIA close-source drivers to
> see if they allow things like
>
>     x = texture(b ? sampler1 : sampler2, t);
>
> If they allow it, I'll submit a spec bug.  If they do not allow it, we
> shouldn't either.
>
Yes, they do [1].  But I don't think it would be a good idea to change
the spec.  For images it would be fine actually (as long as the
condition is dynamically uniform) because they're implemented as a
simple 32-bit integer index.  But for samplers the hell would break
loose because most back-ends expect to be able to determine the sampler
binding statically (up to a dynamically uniform array index if they do
gpu_shader5), so the fact is that it's already broken and will lead to a
crash even if we pretend to allow it.

[1] http://people.freedesktop.org/~currojerez/piglit-summary-image_load_store-kepler/results-all/spec/ARB_shader_image_load_store/compiler/expression-selection.frag.html

>>        ir_constant *cond_val = op[0]->constant_expression_value();
>>        ir_constant *then_val = op[1]->constant_expression_value();
>>        ir_constant *else_val = op[2]->constant_expression_value();
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150201/1fdee89e/attachment.sig>