This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, PR69068] Handle 3-arg phi in copy_bb_and_scalar_dependences


On Mon, 30 May 2016, Tom de Vries wrote:

> On 30/05/16 11:46, Richard Biener wrote:
> > > This patch fixes the assert conservatively by aborting graphite code
> > > >generation when encountering a phi with more than two arguments in
> > > >copy_bb_and_scalar_dependences.
> > > >
> > > >Bootstrapped and reg-tested on x86_64.
> > > >
> > > >OK for trunk, 6 branch?
> 
> > Did you check if simply returning false from bb_contains_loop_phi_nodes
> > instead of asserting works?  The caller has a 'else' that is supposed
> > to handle condition PHIs.  After all it already handles one predecessor
> > specially ...  Thus
> > 
> >    if (EDGE_COUNT (bb->preds) != 2)
> >      return false;
> > 
> > should work here.
> 
> Unfortunately, that doesn't work. We run into another assert in
> copy_cond_phi_nodes:
> ...
>   /* Cond phi nodes should have exactly two arguments.  */
>   gcc_assert (2 == EDGE_COUNT (bb->preds));
> ...

Hah.  So can we still do my suggested change and bail out conservatively
in Cond PHI node handling instead?  Because the PHI node is clearly
_not_ a loop header PHI and the cond phi assert is also a latent bug.

> > Or replace this function with bb_loop_header_p (bb)
> > (works w/o loop info, the function seems to depend on loop info and
> > thus simply checking bb->loop_father->header == bb should work as well).
> 
> I think that will run into the same assert.

Yes.

Richard.

> Thanks,
> - Tom
> 
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]