<p dir="ltr"><br>
On Jul 28, 2015 2:43 AM, "Francisco Jerez" <<a href="mailto:currojerez@riseup.net">currojerez@riseup.net</a>> wrote:<br>
><br>
> The register coalesce pass wasn't rewriting the destination and<br>
> sources of instructions that accessed the second half of a coalesced<br>
> register previously copied with a 16-wide MOV instruction.  E.g.:<br>
><br>
> | ADD (16) vgrf0:f, vgrf0:f, 1.0:f<br>
> | MOV (16) vgrf1:f, vgrf0:f<br>
> | MOV (8)  vgrf2:f, vgrf0+1:f { sechalf }<br>
><br>
> would get incorrectly register-coalesced into:<br>
><br>
> | ADD (16) vgrf1:f, vgrf1:f, 1.0:f<br>
> | MOV (8)  vgrf2:f, vgrf0+1:f { sechalf }<br>
><br>
> The reason is that the mov[i] pointer was being left equal to NULL for<br>
> every other register.  The fact that we've made it to the rewrite loop<br>
> implies that the whole register can been coalesced, so it doesn't seem<br>
> useful to check that mov[i] is non-NULL every time.  Fixes an amount<br>
> of texturing and image_load_store piglit tests on my SIMD-lowering<br>
> branch.</p>
<p dir="ltr">Almost more to the point we don't want to coalesce a register and then not update something that uses it. (which is what was happening.)</p>
<p dir="ltr">Reviewed-by: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>></p>
<p dir="ltr">> ---<br>
>  .../drivers/dri/i965/brw_fs_register_coalesce.cpp  | 27 ++++++++++------------<br>
>  1 file changed, 12 insertions(+), 15 deletions(-)<br>
><br>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_register_coalesce.cpp b/src/mesa/drivers/dri/i965/brw_fs_register_coalesce.cpp<br>
> index 4544122..c078318 100644<br>
> --- a/src/mesa/drivers/dri/i965/brw_fs_register_coalesce.cpp<br>
> +++ b/src/mesa/drivers/dri/i965/brw_fs_register_coalesce.cpp<br>
> @@ -228,7 +228,6 @@ fs_visitor::register_coalesce()<br>
>           continue;<br>
><br>
>        progress = true;<br>
> -      bool was_load_payload = inst->opcode == SHADER_OPCODE_LOAD_PAYLOAD;<br>
><br>
>        for (int i = 0; i < src_size; i++) {<br>
>           if (mov[i]) {<br>
> @@ -243,20 +242,18 @@ fs_visitor::register_coalesce()<br>
><br>
>        foreach_block_and_inst(block, fs_inst, scan_inst, cfg) {<br>
>           for (int i = 0; i < src_size; i++) {<br>
> -            if (mov[i] || was_load_payload) {<br>
> -               if (scan_inst->dst.file == GRF &&<br>
> -                   scan_inst->dst.reg == reg_from &&<br>
> -                   scan_inst->dst.reg_offset == i) {<br>
> -                  scan_inst->dst.reg = reg_to;<br>
> -                  scan_inst->dst.reg_offset = reg_to_offset[i];<br>
> -               }<br>
> -               for (int j = 0; j < scan_inst->sources; j++) {<br>
> -                  if (scan_inst->src[j].file == GRF &&<br>
> -                      scan_inst->src[j].reg == reg_from &&<br>
> -                      scan_inst->src[j].reg_offset == i) {<br>
> -                     scan_inst->src[j].reg = reg_to;<br>
> -                     scan_inst->src[j].reg_offset = reg_to_offset[i];<br>
> -                  }<br>
> +            if (scan_inst->dst.file == GRF &&<br>
> +                scan_inst->dst.reg == reg_from &&<br>
> +                scan_inst->dst.reg_offset == i) {<br>
> +               scan_inst->dst.reg = reg_to;<br>
> +               scan_inst->dst.reg_offset = reg_to_offset[i];<br>
> +            }<br>
> +            for (int j = 0; j < scan_inst->sources; j++) {<br>
> +               if (scan_inst->src[j].file == GRF &&<br>
> +                   scan_inst->src[j].reg == reg_from &&<br>
> +                   scan_inst->src[j].reg_offset == i) {<br>
> +                  scan_inst->src[j].reg = reg_to;<br>
> +                  scan_inst->src[j].reg_offset = reg_to_offset[i];<br>
>                 }<br>
>              }<br>
>           }<br>
> --<br>
> 2.4.6<br>
><br>
</p>