[Mesa-dev] [PATCH] i965/blorb: remove dead code in brw_blorp_const_color_program::compile()

[Pohjolainen, Topi]

On Fri, Apr 22, 2016 at 10:39:37AM -0700, Kenneth Graunke wrote:
> On Friday, April 22, 2016 3:01:27 PM PDT Juha-Pekka Heikkila wrote:
> > base_mrf is unsigned, checking if its greater or equal to zero will
> > not fail.
> > 
> > Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> > ---
> >  src/mesa/drivers/dri/i965/brw_blorp_clear.cpp | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp b/src/mesa/
> drivers/dri/i965/brw_blorp_clear.cpp
> > index 51f915d..06ac69e 100644
> > --- a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> > @@ -314,7 +314,7 @@ brw_blorp_const_color_program::compile(struct 
> brw_context *brw,
> >     /* Now write to the render target and terminate the thread */
> >     brw_fb_WRITE(&func,
> >                  16 /* dispatch_width */,
> > -                base_mrf >= 0 ? brw_message_reg(base_mrf) : mrf_rt_write,
> > +                brw_message_reg(base_mrf),
> >                  brw_null_reg() /* header */,
> >                  msg_type,
> >                  BRW_BLORP_RENDERBUFFER_BINDING_TABLE_INDEX,
> > 
> 
> Yeah, this is definitely dead - base_mrf seems to always equal 2.
> Thanks for catching this :)
> 
> The two parts of the conditional should be equivalent.  One has an
> <8,8,1> region and the other <16,16,1> but those refer to the same
> region, AFAIK.
> 
> I'd be tempted to just use mrf_rt_write unconditionally.  If we go with
> your patch, we should delete mrf_rt_write because it becomes dead.
> 
> Topi, thoughts?

I agree. I think I we can drop it also in
fs_generator::generate_blorp_fb_write(), just need to adjust
brw_blorp_blit_program::render_target_write() accordingly.