<p dir="ltr"><br>
On Apr 22, 2016 10:51 AM, "Pohjolainen, Topi" <<a href="mailto:topi.pohjolainen@intel.com">topi.pohjolainen@intel.com</a>> wrote:<br>
><br>
> On Fri, Apr 22, 2016 at 10:39:37AM -0700, Kenneth Graunke wrote:<br>
> > On Friday, April 22, 2016 3:01:27 PM PDT Juha-Pekka Heikkila wrote:<br>
> > > base_mrf is unsigned, checking if its greater or equal to zero will<br>
> > > not fail.<br>
> > ><br>
> > > Signed-off-by: Juha-Pekka Heikkila <<a href="mailto:juhapekka.heikkila@gmail.com">juhapekka.heikkila@gmail.com</a>><br>
> > > ---<br>
> > >  src/mesa/drivers/dri/i965/brw_blorp_clear.cpp | 2 +-<br>
> > >  1 file changed, 1 insertion(+), 1 deletion(-)<br>
> > ><br>
> > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp b/src/mesa/<br>
> > drivers/dri/i965/brw_blorp_clear.cpp<br>
> > > index 51f915d..06ac69e 100644<br>
> > > --- a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp<br>
> > > +++ b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp<br>
> > > @@ -314,7 +314,7 @@ brw_blorp_const_color_program::compile(struct<br>
> > brw_context *brw,<br>
> > >     /* Now write to the render target and terminate the thread */<br>
> > >     brw_fb_WRITE(&func,<br>
> > >                  16 /* dispatch_width */,<br>
> > > -                base_mrf >= 0 ? brw_message_reg(base_mrf) : mrf_rt_write,<br>
> > > +                brw_message_reg(base_mrf),<br>
> > >                  brw_null_reg() /* header */,<br>
> > >                  msg_type,<br>
> > >                  BRW_BLORP_RENDERBUFFER_BINDING_TABLE_INDEX,<br>
> > ><br>
> ><br>
> > Yeah, this is definitely dead - base_mrf seems to always equal 2.<br>
> > Thanks for catching this :)<br>
> ><br>
> > The two parts of the conditional should be equivalent.  One has an<br>
> > <8,8,1> region and the other <16,16,1> but those refer to the same<br>
> > region, AFAIK.<br>
> ><br>
> > I'd be tempted to just use mrf_rt_write unconditionally.  If we go with<br>
> > your patch, we should delete mrf_rt_write because it becomes dead.<br>
> ><br>
> > Topi, thoughts?<br>
><br>
> I agree. I think I we can drop it also in<br>
> fs_generator::generate_blorp_fb_write(), just need to adjust<br>
> brw_blorp_blit_program::render_target_write() accordingly.</p>
<p dir="ltr">Given that I'm about to rewrite all this code, I wouldn't worry to much about it.  Once I get done with the NIR conversion, generate_blorp_fb_write will get deleted.<br>
</p>