[Mesa-dev] [PATCH] st/mesa: treat a write as a read for range purposes

[Dave Airlie]

On 30 January 2016 at 07:00, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> We use this logic to detect live ranges and then do plain renaming
> across the whole codebase. As such, to prevent WaW hazards, we have to
> treat a write as if it were also a read.
>
> For example, the following sequence was observed before this patch:
>
>  13: UIF TEMP[6].xxxx :0
>  14:   ADD TEMP[6].x, CONST[6].xxxx, -IN[3].yyyy
>  15:   RCP TEMP[7].x, TEMP[3].xxxx
>  16:   MUL TEMP[3].x, TEMP[6].xxxx, TEMP[7].xxxx
>  17:   ADD TEMP[6].x, CONST[7].xxxx, -IN[3].yyyy
>  18:   RCP TEMP[7].x, TEMP[3].xxxx
>  19:   MUL TEMP[4].x, TEMP[6].xxxx, TEMP[7].xxxx
>
> While after this patch it becomes:
>
>  13: UIF TEMP[7].xxxx :0
>  14:   ADD TEMP[7].x, CONST[6].xxxx, -IN[3].yyyy
>  15:   RCP TEMP[8].x, TEMP[3].xxxx
>  16:   MUL TEMP[4].x, TEMP[7].xxxx, TEMP[8].xxxx
>  17:   ADD TEMP[7].x, CONST[7].xxxx, -IN[3].yyyy
>  18:   RCP TEMP[8].x, TEMP[3].xxxx
>  19:   MUL TEMP[5].x, TEMP[7].xxxx, TEMP[8].xxxx
>
> Most importantly note that in the first example, the second RCP is done
> on the result of the MUL while in the second, the second RCP should have
> the same value as the first. Looking at the GLSL source, it is apparent
> that both of the RCP's should have had the same source.
>
> Looking at what's going on, the GLSL looks something like
>
>   float tmin_8;
>   float tmin_10;
>   tmin_10 = tmin_8;
> ... lots of code ...
>   tmin_8 = tmpvar_17;
> ... more code that never looks at tmin_8 ...
>
> And so we end up with a last_read somewhere at the beginning, and a
> first_write somewhere at the bottom. For some reason DCE doesn't remove
> it, but even if that were fixed, DCE doesn't handle 100% of cases, esp
> including loops.
>
> With the last_read somewhere high up, we overwrite the previously
> correct (and large) last_read with a low one, and then proceed to decide
> to merge all kinds of junk onto this temp.
>
> As a result, we should treat a write as a last_read for the purpose of
> determining the live range.
>
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> Cc: mesa-stable at lists.freedesktop.org

I think you've convinced me this is right, I'd like to know why DCE
doesn't trash
that shader better, but I'm happy for this to go in.

Signed-off-by: Dave Airlie <airlied at redhat.com>