[Mesa-dev] [PATCH 3/5] i965: Move TCS output indirect_offset.file check out a level.

[Pohjolainen, Topi]

On Mon, Apr 25, 2016 at 12:33:26PM -0700, Kenneth Graunke wrote:
> On Monday, April 25, 2016 10:46:37 AM PDT Pohjolainen, Topi wrote:
> > On Thu, Apr 21, 2016 at 10:32:07PM -0700, Kenneth Graunke wrote:
> > > I want to add another condition.  Moving the indirect_offset.file
> > > check out a level should make this a little easier.
> > > 
> > > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> > > ---
> > >  src/mesa/drivers/dri/i965/brw_vec4_tcs.cpp | 88 +++++++++++++++
> +--------------
> > >  1 file changed, 46 insertions(+), 42 deletions(-)
> > > 
> > > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_tcs.cpp b/src/mesa/
> drivers/dri/i965/brw_vec4_tcs.cpp
> > > index 8e2713a..8a4ae2e 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_vec4_tcs.cpp
> > > +++ b/src/mesa/drivers/dri/i965/brw_vec4_tcs.cpp
> > > @@ -393,52 +393,56 @@ 
> vec4_tcs_visitor::nir_emit_intrinsic(nir_intrinsic_instr *instr)
> > >        src_reg indirect_offset = get_indirect_offset(instr);
> > >        unsigned imm_offset = instr->const_index[0];
> > >  
> > > -      if (imm_offset == 0 && indirect_offset.file == BAD_FILE) {
> > > -         value.type = BRW_REGISTER_TYPE_F;
> > 
> > I checked this mechanically without understanding the context too well. As
> > such it looks good. I was just wondering if it would have been cleaner to
> > keep the indentation and just:
> > 
> >          if (indirect_offset.file == BAD_FILE) {
> >             emit_urb_write(swizzle(value, swiz), mask,
> >                            imm_offset, indirect_offset);
> >             break;
> >          }
> > 
> >          if (imm_offset == 0) {
> 
> Sure, either way.  It's mostly a matter of taste.  As is, we select
> offsets/swizzles/masks as necessary, then do a single write in all cases.
> With that change, we'd have indirects and passthrough do a write and
> stop, otherwise fall through and handle directs/tesslevel via a separate
> write.
> 
> It would definitely have made for a smaller diff, but I think I have a
> slight preference for the code as is.  It doesn't really matter though;
> I'm happy to change it if you'd like.

I'm fine either way, your call. I couldn't anything amiss in the original, so:

Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>