[Mesa-dev] [PATCH] Move all of the var decls to the front of the IR list in reverse order.

[Ian Romanick]

On 06/11/2013 01:27 PM, Chad Versace wrote:
> On 06/08/2013 01:05 PM, Ian Romanick wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> This has the (intended!) side effect that vertex shader inputs and
>> fragment shader outputs will appear in the IR in the same order that
>> they appeared in the shader code.  This results in the locations being
>> assigned in the declared order.  Many (arguably buggy) applications
>> depend on this behavior, and it matches what nearly all other drivers
>> do.
>>
>> Fixes the (new) piglit test attrib-assignments.
>>
>> NOTE: This is a candidate for stable release branches.
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> Cc: Chad Versace <chad.versace at linux.intel.com>
>> ---
>>   src/glsl/ast_to_hir.cpp | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>> index e918ade..886cec5 100644
>> --- a/src/glsl/ast_to_hir.cpp
>> +++ b/src/glsl/ast_to_hir.cpp
>> @@ -94,6 +94,24 @@ _mesa_ast_to_hir(exec_list *instructions, struct
>> _mesa_glsl_parse_state *state)
>>      detect_conflicting_assignments(state, instructions);
>>
>>      state->toplevel_ir = NULL;
>> +
>> +   /* Move all of the variable declarations to the front of the IR
>> list, and
>> +    * reverse the order.  This has the (intended!) side effect that
>> vertex
>> +    * shader inputs and fragment shader outputs will appear in the IR
>> in the
>> +    * same order that they appeared in the shader code.  This results
>> in the
>> +    * locations being assigned in the declared order.  Many (arguably
>> buggy)
>> +    * applications depend on this behavior, and it matches what
>> nearly all
>> +    * other drivers do.
>> +    */
>> +   foreach_list_safe(node, instructions) {
>> +      ir_variable *const var = ((ir_instruction *) node)->as_variable();
>> +
>> +      if (var == NULL)
>> +     continue;
>> +
>> +      var->remove();
>> +      instructions->push_head(var);
>> +   }
>>   }
>
> Didn't you mention that this breaks some gles2conform test?

Yes.  GL2FixedTests/stencil_plane_operation/stencil_plane_operation.test 
breaks.  However, I think this is a pre-existing bug.  If I edit its 
shader to list the attributes in the opposite order, the test breaks 
/without/ this patch.  I've debugged it a little bit, but I don't have 
any clues yet.

> I verified that this fixes Pool Master Pro and gives no
> regressions on Android's gles2conform at 64x64 and all EGLConfigs.

So... stencil_plane_operation passes on Android with and without this 
patch?  confusion++;

> Tested-by: Chad Versace <chad.versace at linux.intel.com>