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

[Chad Versace]

On 06/11/2013 01:39 PM, Ian Romanick wrote:
> 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>

I have never seen stencil_plane_operation fail on Android. For the record,
I'm using Harris Beach (Haswell Mobile ULT GT3u).