On Tue, Mar 6, 2018 at 4:41 AM, Jeff Law <l...@redhat.com> wrote: > 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.
While it's true that the hardreg is set by the callee the GIMPLE IL indeed doesn't reflect this (and we have a similar issue with EH where the exceptional return does _not_ include the assignment to the LHS but the GIMPLE IL does...). So with your patch we should see ret_1 = setjmp (); | \ | AB dispatcher | / v v # ret_2 = PHI <ret_1, ret_1(ab)> ... even w/o a PHI. So I think we should be fine given we have that edge from setjmp to the abnormal dispatcher. Richard. > > > > Jeff
