[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:57 PM, Chad Versace wrote:
> 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).

By the way, please add the "glsl: " prefix to the commit subject.