[Mesa-dev] [PATCH] i965/vec4: Fix liveness analysis with BRW_OPCODE_SEL

[Francisco Jerez]

Iago Toral Quiroga <itoral at igalia.com> writes:

> We only consider a vgrf defined by a given block if the block writes to it
> unconditionally. So far we have been checking this by testing that the
> instruction is not predicated, however, in the case of BRW_OPCODE_SEL,
> the predication is used to select the value to write, not to decide if
> the write is actually done. The consequence of this was increased life
> spans for affected vgrfs, which could lead to additional register pressure.
>
> Since NIR generates selects for conditional writes this was causing massive
> register pressure in a handful of piglit and dEQP tests that had a large
> number of select operations with the NIR-vec4 backend.
>
> Fixes the following piglit tests with the NIR-vec4 backend:
> spec/glsl-1.50/execution/variable-indexing/vs-output-array-vec4-index-wr-before-gs
> spec/glsl-1.50/execution/variable-indexing/gs-input-array-vec4-index-rd
> spec/glsl-1.50/execution/variable-indexing/vs-output-array-vec2-index-wr-before-gs
> spec/glsl-1.50/execution/variable-indexing/vs-output-array-vec3-index-wr-before-gs
> spec/glsl-1.50/execution/variable-indexing/vs-output-array-float-index-wr-before-gs
>
> Fixes the 80 dEQP tests with the NIR-vec4 backend in the following category:
> dEQP-GLES3.functional.ubo.*
> ---
>  src/mesa/drivers/dri/i965/brw_vec4_live_variables.cpp | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_live_variables.cpp b/src/mesa/drivers/dri/i965/brw_vec4_live_variables.cpp
> index 29b4a53..cc688ef 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_live_variables.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_live_variables.cpp
> @@ -96,7 +96,8 @@ vec4_live_variables::setup_def_use()
>  	  * are the things that screen off preceding definitions of a
>  	  * variable, and thus qualify for being in def[].
>  	  */
> -	 if (inst->dst.file == GRF && !inst->predicate) {
> +	 if (inst->dst.file == GRF &&
> +	     (!inst->predicate || inst->opcode == BRW_OPCODE_SEL)) {
>              for (unsigned i = 0; i < inst->regs_written; i++) {
>                 for (int c = 0; c < 4; c++) {
>                    if (inst->dst.writemask & (1 << c)) {

Looks good, thanks,

Reviewed-by: Francisco Jerez <currojerez at riseup.net>

> -- 
> 1.9.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150720/f7d281d7/attachment.sig>