[Mesa-dev] Issue with 3008161d28: st_glsl_to_tgsi: rewrite rename registers to use array fully.

[Brian Paul]

On 07/27/2017 05:52 PM, Dave Airlie wrote:
> On 28 July 2017 at 09:37, Brian Paul <brianp at vmware.com> wrote:
>> Hi Dave,
>>
>> We found an issue with this change:
>>
>> commit 3008161d28e38336ba39aba4769a2deaf9732f55
>> Author: Dave Airlie <airlied at redhat.com>
>> Date:   Tue May 30 15:52:13 2017 +1000
>>
>>      st_glsl_to_tgsi: rewrite rename registers to use array fully.
>>
>>      Instead of having to search the whole array, just use the whole
>>      thing and store a valid bit in there with the rename.
>>
>>      Removes this from the profile on some of the fp64 tests
>>
>>      Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>
>>      Signed-off-by: Dave Airlie <airlied at redhat.com>
>>
>> After this patch landed, the number of temporary registers used by the TGSI
>> code increased.  With a particular Photoshop shader, the number increased
>> from 26 to 31.  With our legacy virtual GPU version, this causes us to run
>> out of registers and fail translation.
>
>>
>> Did you expect this change to increase the number of temps used?  Is it
>> possible to get the old behavior back?  I don't think we can just revert
>> this patch since a number of changes since then build on it.
>
> This shouldn't have caused any increase in the number of registers used at all,
> it should have just improved the memory usage pattern.
>
> We could probably revert it, I don't think that much changed, it
> reverts cleanly,

Yeah, but it doesn't compile after reverting.

../../../src/mesa/state_tracker/st_glsl_to_tgsi.cpp: In member function 
'void glsl_to_tgsi_visitor::rename_temp_registers(int, rename_reg_pair*)':
../../../src/mesa/state_tracker/st_glsl_to_tgsi.cpp:4859:31: error: 
'struct rename_reg_pair' has no member named 'valid'
           if (renames[old_idx].valid)
                                ^
make[5]: *** [state_tracker/st_glsl_to_tgsi.lo] Error 1

> but I'd really like an apitrace or piglit that shows the problem so I
> can work out how where it goes wrong.

Sure, I'll try to do that...

-Brian