[Mesa-dev] [PATCH] glsl/linker: Allow unused in blocks which are not declated on previous stage

[Alejandro Piñeiro]

On 21/08/18 03:02, Timothy Arceri wrote:
> On 20/08/18 23:31, vadym.shovkoplias wrote:
>>  From Section 4.3.4 (Inputs) of the GLSL 1.50 spec:
>>
>>      "Only the input variables that are actually read need to be written
>>       by the previous stage; it is allowed to have superfluous
>>       declarations of input variables."
>>
>> Fixes:
>>      * interstage-multiple-shader-objects.shader_test
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101247
>> Signed-off-by: Vadym Shovkoplias <vadym.shovkoplias at globallogic.com>
>> ---
>>   src/compiler/glsl/link_interface_blocks.cpp | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/compiler/glsl/link_interface_blocks.cpp
>> b/src/compiler/glsl/link_interface_blocks.cpp
>> index e5eca9460e..801fbcd5d9 100644
>> --- a/src/compiler/glsl/link_interface_blocks.cpp
>> +++ b/src/compiler/glsl/link_interface_blocks.cpp
>> @@ -417,9 +417,15 @@ validate_interstage_inout_blocks(struct
>> gl_shader_program *prog,
>>          * write to any of the pre-defined outputs (e.g. if the
>> vertex shader
>>          * does not write to gl_Position, etc), which is allowed and
>> results in
>>          * undefined behavior.
>> +       *
>> +       * From Section 4.3.4 (Inputs) of the GLSL 1.50 spec:
>> +       *
>> +       *    "Only the input variables that are actually read need to
>> be written
>> +       *     by the previous stage; it is allowed to have superfluous
>> +       *     declarations of input variables."
>>          */
>>         if (producer_def == NULL &&
>> -          !is_builtin_gl_in_block(var, consumer->Stage)) {
>> +          !is_builtin_gl_in_block(var, consumer->Stage) &&
>> var->data.used) {
>
> This concerns me a little. As far as I remember 'used' was added to
> make compiler warning better but it's not 100% reliable.

+1 on the concerns thing. In addition to be used mostly for warnings, we
need to take into account his description comment at ir.h:

     "
       * This is only maintained in the ast_to_hir.cpp path, not in
       * Mesa's fixed function or ARB program paths.
     "

So if we start to use this while linking, then we need to ensure that it
is maintained outside the ast_to_hir pass (or at least, ensure that it
is correct after that pass). And if we got that, then that comment
became obsolete, and should be removed as part of the patch.

>
>>            linker_error(prog, "Input block `%s' is not an output of "
>>                         "the previous stage\n",
>> var->get_interface_type()->name);
>>            return;
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev