r230255 - Only lower __builtin_setjmp / __builtin_longjmp to

[Hal Finkel]

----- Original Message -----
> From: "John McCall" <rjmccall at gmail.com>
> To: "John McCall" <rjmccall at gmail.com>, "Hal Finkel" <hfinkel at anl.gov>, "<hans at hanshq.net>" <hans at hanshq.net>,
> "cfe-commits" <cfe-commits at cs.uiuc.edu>
> Sent: Tuesday, March 3, 2015 6:00:58 PM
> Subject: Re: r230255 - Only lower __builtin_setjmp / __builtin_longjmp to
> 
> 
> On Tue, Mar 3, 2015 at 7:16 AM, Joerg Sonnenberger <
> joerg at britannica.bec.de > wrote:
> 
> On Tue, Mar 03, 2015 at 12:05:53AM -0800, John McCall wrote:
> > GCC implements this builtin with much more generality than we do,
> > probably because GCC predates widespread adoption of libUnwind and
> > we
> > don't. Hal's comment about not trying to match GCC on PPC aside
> > (and
> > I find that comment pretty troubling!), my understanding of the
> > general
> > intent of this builtin is that's for implementing extremely
> > lightweight
> > context-saving (on the assumption that the platform doesn't have
> > that
> > many callee-save registers; it would probably be awful on e.g.
> > AArch64)
> > and that the amount of state it saves is so small (PC, SP, FP if
> > it's
> > not rederivable from SP) that there's no reason [i]not[/i] to match
> > GCC.
> 
> Actually, it is a bit more complicated. A good setjmp/longjmp
> implementation in libc saves exactly the same set of registers. The
> only
> reason the GCC implementation can be better is that it forces
> register
> spilling to fixed locations and makes longjmp depend on that. I'm not
> even sure there is any optimisation for detecting that longjmp
> doesn't
> need to restore all registers.
> 
> 
> 
> This is not how __builtin_setjmp / __builtin_longjmp work at all.
> __builtin_longjmp only restores enough to restore the original call
> frame and IP; the __builtin_setjmp site is responsible for restoring
> the other registers, at its own pace and with its own
> function-specific
> information.
> 
> 
> setjmp and longjmp are also specced to preserve more than just
> register state. A target that decides to use the library function
> should
> at least be lowering to _setjmp/_longjmp.
> 
> 
> 
> "Implement it" comes with a quite a cost as it requires implementing
> a
> good chunk of SJ/LJ functionality AND detecting when the landing pads
> need to be added, even if no exceptions are used. That's why ARM
> fails
> currently even on Darwin, which does use SJ/LJ for exception
> handling.
> 
> 
> 
> Okay. I'm quite ready to believe that the current intrinsics are
> excessively
> tied to the needs of EH lowering. If implementing this properly on
> ARM
> is unreasonable in the current representation, so be it.
> 
> 
> From what I have seen, I don't believe in a performance benefit
> compared
> to a proper libc implementation. That was already a fallacity of HP's
> libunwind -- for the platforms at the time, only Itanium had an
> insane
> enough callee-save register set, mostly the FP registert. But that's
> not
> gong to help with SJ/LJ in this context either.
> 
> 
> 
> This is not what people have found empirically, at all. The
> performance
> impact is quite drastic on targets with a small callee-save register
> set,
> because every register save / restore that can be avoided or
> optimized
> in the setjmp'ing function has a significant impact.
> 
> 
> It also allows the bulk of the saves to be hoisted to the top of the
> setjmp'ing function instead of being done at setjmp time.
> 
> 
> 
> > But if you can get the LLVM list to agree with Hal that
> > compatibility
> > (even across compiler versions) is a non-goal, and you can show me
> > more
> > convincingly that e.g. Ruby only uses this in their core
> > implementation
> > and it is not used directly in the compiled code of native gems,
> > then
> > I am willing to change my mind.
> 
> As I said, I don't care about ABI compatibility enough to spend the
> time
> to fix all backends. What I do care is that we currently silently
> produce obviously bad code.
> 
> 
> Right, I agree that this is a serious problem.
> 
> 
> If the ABI contract is supposed to be "uses 6 longs or less", which
> even
> 
> GCC doesn't seem to get right, I am perfectly fine with just creating
> a
> 
> failure in this case. That should be a trivial change.
> 
> 
> I think the best solution is for us to not claim to support
> __builtin_setjmp
> and __builtin_longjmp on targets that don't support them. That way we
> avoid making any particular ABI commitment in the absence of an
> optimized
> implementation; even if you don't care about GCC compatibility, you
> should
> care about compatibility between clang versions. And an error here
> would
> simply disable the feature in Ruby's configure check.

This is a good point. I second this.

 -Hal

> 
> 
> It sounds like you've already investigated which targets actually
> support
> the intrinsic. Just move that to a flag on the AST-level TargetInfo
> and make
> the custom type-checking for __builtin_setjmp / __builtin_longjmp in
> SemaChecking.cpp reject calls on targets that don't support it.
> Targets
> that want to optimize this can just implement the intrinsic (or a
> different,
> less EH-linked intrinsic) and flip the switch in their TargetInfo.
> 
> 
> John.

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory