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

[Kenneth Graunke]

On Monday, June 09, 2014 11:29:35 AM Iago Toral wrote:
> 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

Yeah, that sounds good.  Feel free to put my Reviewed-by on such a patch.

Thanks!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140609/a3cffb46/attachment-0001.sig>