Re: [RFA][PATCH][PR middle-end/61118] Improve tree CFG accuracy for setjmp/longjmp

[Jeff Law <law at redhat dot com>]

On 03/05/2018 12:30 PM, Michael Matz wrote:
> Hi,
> 
> On Mon, 5 Mar 2018, Jeff Law wrote:
> 
>>>> The single successor test was strictly my paranoia WRT abnormal/EH 
>>>> edges.
>>>>
>>>> I don't immediately see why the CFG would be incorrect if the 
>>>> successor of the setjmp block has multiple preds.
>>>
>>> Actually, without further conditions I don't see how it would be safe 
>>> for the successor to have multiple preds.  We might have this 
>>> situation:
>>>
>>> bb1: ret = setjmp
>>> bb2: x0 = phi <x1 (bb1), foo(bbX)>
>> No.  Can't happen -- we're still building the initial CFG.  There are no
>> PHI nodes, there are no SSA_NAMEs.
> 
> While that is currently true I think it's short-sighted.  Thinking about 
> the situation in terms of SSA names and PHI nodes clears up the mind.  In 
> addition there is already code which builds (sub-)cfgs when SSA form 
> exists (gimple_find_sub_bbs).  Currently that code can't ever generate 
> setjmp calls, so it's not an issue.
It's not clearing up anything for me.  Clearly you're onto something
that I'm missing, but still trying to figure out.

Certainly we have to be careful WRT the implicit set of the return value
of the setjmp call that occurs on the longjmp path.  That's worth
investigating.  I suspect that works today more by accident of having an
incorrect CFG than by design.


> 
>> We have two choices we can either target the setjmp itself, which is
>> what we've been doing and is inaccurate.  Or we can target the point
>> immediately after the setjmp, which is accurate.
> 
> Not precisely, because the setting of the return value of setjmp does 
> happen after both returns.  So moving the whole second-return edge target 
> to after the setjmp call (when it includes an LHS) is not correct 
> (irrespective how the situation in the successor BBs like like).
But it does or at least it should.  It's implicitly set on the longjmp
side.  If we get this wrong I'd expect we'll see uninit uses in the PHI.
 That's easy enough to instrument and check for.

This aspect of setjmp/longjmp is, in some ways, easier to see in RTL
because the call returns its value in a hard reg which is implicitly set
by the longjmp and we immediately copy it into a pseudo.   Which would
magically DTRT if we had the longjmp edge target the point just after
the setjmp in RTL.




Jeff