Re: [PATCH] Fix more PR80928 fallout

[Jeff Law <law at redhat dot com>]

On 06/23/2017 05:39 AM, Richard Biener wrote:
> 
> SLP induction vectorization runs into the issue that it remembers
> pointers to PHI nodes in the SLP tree during analysis.  But those
> may get invalidated by loop copying (for prologue/epilogue peeling
> or versioning) as the low-level CFG helper copy_bbs works in the
> way of copying individual BBs plus their outgoing edges but with
> old destinations and at the end re-directing the edges to the
> desired location.  In SSA this triggers the whole machinery of
> making room for new PHI nodes -- that is undesirable because it
> causes re-allocation of PHI nodes in the set of source blocks.
> 
> After much pondering I arrived at the following (least ugly) solution
> to this "problem" (well, I define it as a problem, it's at least
> an inefficiency and a workaround in the vectorizer would be way
> uglier).  Namely simply do not trigger the SSA machinery for
> blocks with BB_DUPLICATED (I skimmed all other users and they seem
> fine).
> 
> In the process I also implemented some poisoning of the old PHI node
> when we reallocate (well, free) PHI nodes.  But that triggers some
> other issues, one fixed by the tree-ssa-phionlycoprop.c hunk below.
> So I'm not submitting it as part of this fix.
> 
> Bootstrapped (with the poisoning sofar, plain patch still running)
> on x86_64-unknown-linux-gnu, testing in progress.
> 
> Comments welcome, testing won't finish before I leave for the
> weekend.
I fully support poisoning the old PHI nodes -- I tracked down a similar
problem just a few months back that probably would have been obvious if
we had poisoned the old nodes (79621 which is now a missed optimization
bug).

I wouldn't be surprised if there's others lurking and given the general
trend of using block duplication to enable various optimizations,
catching this stuff early would definitely be good.

Jeff