On Wed, Mar 4, 2015 at 1:41 PM, Richard Biener <richard.guent...@gmail.com> wrote: > On Wed, Mar 4, 2015 at 6:27 AM, Jeff Law <l...@redhat.com> wrote: >> On 03/02/15 01:38, Richard Biener wrote: >>> >>> On Mon, Mar 2, 2015 at 6:34 AM, Aldy Hernandez <al...@redhat.com> wrote: >>>> >>>> As I mention in the PR... >>>> >>>> What's happening here is that the ipa_polymorphic_call_context >>>> constructor >>>> is calling walk_ssa_copies on a PHI node that has no arguments. This >>>> happens because finalize_jump_threads eventually removes some PHI >>>> arguments >>>> as it's redirecting some edges, leaving a PHI with no arguments: >>>> >>>> SR.33_23 = PHI <> >>>> >>>> This should get cleaned up later, but the IPA polymorphic code gets >>>> called >>>> during the actual CFG clean-up, and walk_ssa_copies cannot handle an >>>> empty >>>> PHI. >>>> >>>> Approved by Honza. >>>> >>>> Fully tested on x86-64 Linux and verified that the patch fixes the ICE on >>>> an >>>> x86-64 Linux cross aarch64-linux-gnu cc1plus. >>>> >>>> Committed to mainline. >>> >>> >>> I think the real issue is that the walking code is executed via fold_stmt >>> when >>> called with an API that tells you not to walk SSA use-def chains. >> >> ? We have something that tells us not to walk the chains? I don't see it >> in an API for fold_stmt. How is the ipa-polymorphic code supposed to know >> when it can't follow the chains? > > It gets passed the valueize callback now which returns NULL_TREE for > SSA names we can't follow.
Btw, for match-and-simplify I had to use that as default for fold_stmt
_exactly_ because of the call to fold_stmt from replace_uses_by
via merge-blocks from cfgcleanup. This is because replace-uses-by
doesn't have all uses replaced before it folds the stmt!
We also have the "weaker" in-place flag.
>> The restrictions on what we can do while we're in the inconsistent state
>> prior to updating the ssa graph aren't defined anywhere and I doubt anyone
>> really knows what they are. That's obviously concerning.
>>
>> We might consider trying to narrow the window in which these inconsistencies
>> are allowed. To do that I think we need to split cfgcleanup into two
>> distinct parts. First is unreachable block removal (which is needed so that
>> we can compute the dominators). Second is everything else.
>>
>> The order of operations would be something like
>>
>> remove unreachable blocks
>> ssa graph update
>> rest of cfg_cleanup
>>
>> That just feels too intrusive to try at this stage though.
>
> Well, not folding statements from cfg-cleanup would be better.
>
> I'll have a look at the testcase in the PR and will come back with a
> suggestion on what to do for GCC 5.
I'd say that the devirtualization code is quite a heavy thing do to from
fold_stmt. Yes - it want's to catch all cases if a stmt is modified
(after which passes should fold it).
So I am testing the following on x86_64 (verified it fixes the testcase
with a aarch64 cross).
Richard.
2015-03-04 Richard Biener <rguent...@suse.de>
PR middle-end/65233
* ipa-polymorphic-call.c: Include tree-ssa-operands.h and
tree-into-ssa.h.
(walk_ssa_copies): Revert last chage. Instead do not walk
SSA names registered for SSA update.
fix-pr65233
Description: Binary data
