[Mesa-dev] [PATCH v2 1/4] i965: Always set a valid block end pointer

[Iago Toral]

On Mon, 2014-06-09 at 02:22 -0700, Kenneth Graunke wrote:
> On Thursday, June 05, 2014 03:03:05 PM Iago Toral Quiroga wrote:
> > When a instruction stream ends in a block structure (like a IF/ELSE/ENDIF) 
> the
> > last block's end pointer will not be set, leading to a crash later on in
> > fs_live_variables::setup_def_use().
> > 
> > If we have not assigned the end pointer of the last block, set it to the 
> last
> > instruction.
> > ---
> >  src/mesa/drivers/dri/i965/brw_cfg.cpp | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_cfg.cpp 
> b/src/mesa/drivers/dri/i965/brw_cfg.cpp
> > index 6bf99f1..d4647c4 100644
> > --- a/src/mesa/drivers/dri/i965/brw_cfg.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_cfg.cpp
> > @@ -257,6 +257,11 @@ cfg_t::cfg_t(exec_list *instructions)
> >        }
> >     }
> >  
> > +   /* If the instruction stream ended with a block structure we need to
> > +      set the block's end pointer to the last instruction here */
> > +   if (!cur->end)
> > +      cur->end = (backend_instruction *)instructions->get_tail();
> > +
> >     cur->end_ip = ip;
> >  
> >     make_block_array();
> > 
> 
> I'm not too excited about this patch - threads must terminate with a SEND 
> message to a particular shared function, such as a URB write or FB write.  So 
> the instruction stream really shouldn't end in an IF/ELSE/ENDIF.
> 
> I think we should probably just drop it, as it isn't needed for the latest 
> version of your series anyway.

Agreed.

> That said, perhaps we should add an assert(cur->end) here?  It seems like 
> catching this problem earlier than fs_live_variables::setup_def_use would be 
> nice.  What do you think?

Yes, that would be better.

Should I just push a commit adding the assertion then? I guess there is
not much to review about it :)

Iago