Re: [PATCH]: Fix PR26725: ICE in check_reg_live, at haifa-sched.c:4645

[Richard Sandiford <richard at codesourcery dot com>]

Maxim Kuvyrkov <mkuvyrkov@ispras.ru> writes:
>> --------------------------------------------------------------------------
>> static basic_block
>> rtl_redirect_edge_and_branch_force (edge e, basic_block target)
>> {
>>   if (redirect_edge_and_branch (e, target)
>>       || e->dest == target)
>>     return NULL;
>> 
>>   /* In case the edge redirection failed, try to force it to be non-fallthru
>>      and redirect newly created simplejump.  */
>>   return force_nonfallthru_and_redirect (e, target);
>> }
>> --------------------------------------------------------------------------
>> 
>> I.e., force_nonfallthru_and_redirect does not set BB_DIRTY if E's
>> target changes.
>> 
>> force_nonfallthru_and_redirect is used by two functions: force_nonfallthru
>> and the one quoted above.  The former always passes e->dest as the second
>> argument, so I think it makes more sense to set BB_DIRTY in the caller
>> (rtl_redirect_edge_and_branch_force) rather than the callee.  This also
>
> caller <-> callee :)

No.  caller == rtl_redirect_edge_and_branch_force, callee ==
force_nonfallthru_and_redirect.  And I'm adding the code in the
caller rather than the callee.

>> makes the link between BB_DIRTY and the redirect_edge_and_branch functions
>> more obvious.
>
> I agree that your fix is more obvious and clear than mine.  The reason 
> why I've chosen not to fix rtl_redirect_edge_and_branch_force is that 
> BB_DIRTY is 'set if insns in BB are modified'. 
> force_nonfallthru_and_redirect doesn't modify e->src, so, I thought, 
> there is no enough argument to set BB_DIRTY for e->src anytime 
> rtl_redirect_edge_and_branch_force is invoked.

I don't really follow.  force_nonfallthru_and_redirect (as used by
rtl_redirect_edge_and_branch_force) will change the jump at the end
of e->src, so I'm not sure why you say it "doesn't modify e->src".

>> Bootstrapped & regression tested on mips64-linux-gnu.  I checked that
>> it fixes the immediate point of failure for the original testcase on
>> an i686-pc-linux-gnu to ia64-linux-gnu cross compiler, but it then
>> segfaults for unrelated reasons in ia64_reorg.
>
> The segfault is PR26743 and there is a patch pending for it.  Have you 
> bootstrapped / regtested your patch on i686-linux-gnu?  I've 
> successfully tested mine, but yours is more substantial.

I'm not sure why you think that. ;)  But yeah, I'll bootstrap on
i686-linux-gnu as well if you like.

Richard