On Sun, Jan 31, 2016 at 7:29 PM, Dave Airlie <airl...@gmail.com> wrote: > On 30 January 2016 at 07:00, Ilia Mirkin <imir...@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 <imir...@alum.mit.edu> >> Cc: mesa-sta...@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 <airl...@redhat.com>
Thanks! I'm going to leave this on the list until tomorrow in case anyone else has feedback... these things are subtle and I don't want to be doing the wrong thing. However I really do believe it's the correct fix to deal with WaW hazards that otherwise appear. -ilia _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
