r230255 - Only lower __builtin_setjmp / __builtin_longjmp to

[John McCall]

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.

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150303/cda4dc51/attachment.html>