[Mesa-dev] [PATCH v2 2/6] i965/vec4: Remove checks for reladdr when checking for spillable registers

[Francisco Jerez]

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

> In theory, GRF array access should have been moved to scratch by the time
> we got here, so this should never happen. A full piglit run forcing
> spilling of all registers seems to confirm this. The FS backend
> does not seem to check for this either.
> ---
>  src/mesa/drivers/dri/i965/brw_vec4_reg_allocate.cpp | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_reg_allocate.cpp b/src/mesa/drivers/dri/i965/brw_vec4_reg_allocate.cpp
> index a9bf0d8..cff5406 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_reg_allocate.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_reg_allocate.cpp
> @@ -282,15 +282,13 @@ vec4_visitor::evaluate_spill_costs(float *spill_costs, bool *no_spill)
>        for (unsigned int i = 0; i < 3; i++) {
>  	 if (inst->src[i].file == GRF) {
>  	    spill_costs[inst->src[i].reg] += loop_scale;
> -            if (inst->src[i].reladdr)
> -               no_spill[inst->src[i].reg] = true;
> +	    assert(!inst->src[i].reladdr);
>  	 }
>        }
>  
>        if (inst->dst.file == GRF) {
>  	 spill_costs[inst->dst.reg] += loop_scale;
> -         if (inst->dst.reladdr)
> -            no_spill[inst->dst.reg] = true;

I guess the intention behind these checks was to support unlowered
indirect addressing of GRFs sometime in the future.  Is there some
reason why you want to remove them?

That said, I don't think there would be any reason to force indirectly
addressed VGRFs not to spill even if we implemented them, because if the
VGRF is larger than one physical register it will already have been
marked as no-spill by the previous loop at the top of this function, and
if they are exactly one physical register the reladdr value is
guaranteed to be zero so the indirect addressing is a no-op.

How about we get rid of these checks as you have done, but leave no
assertions behind?

> +	 assert(!inst->dst.reladdr);
>        }
>  
>        switch (inst->opcode) {
> -- 
> 1.9.1
-------------- 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/20150730/af38f52c/attachment.sig>