[Mesa-dev] [PATCH 14/18] i965/fs: Don't assign high regs for CS termination send

[Jordan Justen]

On 2015-03-16 13:49:29, Kenneth Graunke wrote:
> On Saturday, March 14, 2015 09:54:24 PM Jordan Justen wrote:
> > Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> > ---
> >  src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp b/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp
> > index 1f30a19..fe3406c 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp
> > @@ -598,7 +598,7 @@ fs_visitor::assign_regs(bool allow_spilling)
> >            * We could just do "something high".  Instead, we just pick the
> >            * highest register that works.
> >            */
> > -         if (inst->eot) {
> > +         if (stage != MESA_SHADER_COMPUTE && inst->eot) {
> >              int size = alloc.sizes[inst->src[0].reg];
> >              int reg = screen->wm_reg_sets[rsi].class_to_ra_reg_range[size] - 1;
> >              ra_set_node_reg(g, inst->src[0].reg, reg);
> > 
> 
> This looks wrong to me - the ISA reference for the SEND instruction
> says:
> 
> "A root thread originated from the media (generic) pipeline must terminate
>  with a send instruction with message to the Thread Spawner unit. A
>  child thread should also terminate with a send to TS. [...]
> 
>  The send with EOT should use register space R112-R127 for <src>."
> 
> which implies to me that this restriction is for any SEND with EOT,
> including those sent to the thread spawner for compute.
> 
> Is there a particular need for this patch, or were you just trying to
> remove unnecessary restrictions?

I think the original motivation was to avoid the 'assert(!inst->eot);'
that you have since removed in 10d8a1a8.

I wrote it this way (and kept it around) because the comment ("When we
do send-from-GRF for FB writes, we need to ensure that the last write
instruction sends from a high register.") didn't sound like something
applicable to CS.

So, I expect that thanks to your 10d8a1a8 patch I can drop this one.

That said, I did try removing it, and hit some issue in register
allocation, so it looks like something else is needed in its place...

Thanks,

-Jordan