I talked to both Geoff and Keith about this in the last couple of days, and it's a bit gross how we do this now. We should probably just having something like: struct NoTailCalls { ~NoTailCalls() { m_bit = false; } volatile bool m_bit; }; And then a macro like: #define NO_TAIL_CALLS() NoTailCalls noTailCalls ## __counter__ And then inside your function, you can do: static void foo() { NO_TAIL_CALLS(); ...; }
Yeah, it looks like at least clang respects the NoTailCalls class so this seems like a much better solution.
I've ensured that this works with GCC. #define NEVER_INLINE __attribute__((__noinline__)) struct NoTailCalls { ~NoTailCalls() { m_bit = false; } volatile bool m_bit; }; NEVER_INLINE int target2() { return 42; } NEVER_INLINE int target() { NoTailCalls noTailCalls; return target2(); } int main(int argc, char const* argv[]) { target(); return 0; } ================ target function with -O3 _Z6targetv: .LFB4: .cfi_startproc subq $16, %rsp .cfi_def_cfa_offset 24 call _Z7target2v movb $0, 15(%rsp) addq $16, %rsp .cfi_def_cfa_offset 8 ret .cfi_endproc Without NoTailCalls, it becomes _Z6targetv: .LFB4: .cfi_startproc jmp _Z7target2v .cfi_endproc
(In reply to Yusuke Suzuki from comment #2) > I've ensured that this works with GCC. > > #define NEVER_INLINE __attribute__((__noinline__)) > > struct NoTailCalls { > ~NoTailCalls() { m_bit = false; } I bet you just need this to call compilerFence(). > volatile bool m_bit; Then you don't need this. > }; > > NEVER_INLINE int target2() > { > return 42; > } > > NEVER_INLINE int target() > { > NoTailCalls noTailCalls; > return target2(); > } > > int main(int argc, char const* argv[]) > { > target(); > return 0; > } > > > ================ target function with -O3 > > _Z6targetv: > .LFB4: > .cfi_startproc > subq $16, %rsp > .cfi_def_cfa_offset 24 > call _Z7target2v > movb $0, 15(%rsp) > addq $16, %rsp > .cfi_def_cfa_offset 8 > ret > .cfi_endproc > > Without NoTailCalls, it becomes > > _Z6targetv: > .LFB4: > .cfi_startproc > jmp _Z7target2v > .cfi_endproc
Don and Alex, Could you test whether this compilerFence() / `volatile bool` NoTailCalls works on Windows (with MSVC)? If it is ok, NoTailCalls becomes platform-independent way to do this :)
(In reply to Filip Pizlo from comment #3) > (In reply to Yusuke Suzuki from comment #2) > > I've ensured that this works with GCC. > > > > #define NEVER_INLINE __attribute__((__noinline__)) > > > > struct NoTailCalls { > > ~NoTailCalls() { m_bit = false; } > > I bet you just need this to call compilerFence(). > > > volatile bool m_bit; > > Then you don't need this. Nice. I've just checked it on GCC in https://godbolt.org. #define NEVER_INLINE __attribute__((__noinline__)) struct NoTailCalls { ~NoTailCalls() { asm volatile ("" ::: "memory"); } }; NEVER_INLINE int target2() { return 42; } NEVER_INLINE int target() { NoTailCalls noTailCalls; return target2(); } int main(int argc, char const* argv[]) { target(); return 0; } ============== target2(): mov eax, 42 ret target(): call target2() ret main: call target() xor eax, eax ret Yeah, it seems good.
On clang you should use __attribute__((disable_tail_calls))
(In reply to JF Bastien from comment #6) > On clang you should use __attribute__((disable_tail_calls)) That’s not what we want here. That disables a callee from being tail called. It doesn’t defend against a caller performing a tail call
Created attachment 320847 [details] patch
Comment on attachment 320847 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=320847&action=review > Source/WTF/wtf/NoTailCalls.h:55 > +#define NO_TAIL_CALLS() WTF::NoTailCalls _noTailCalls_ An alternate name could be: DONT_TAIL_CALL
Comment on attachment 320847 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=320847&action=review r=me with nit. > Source/WTF/wtf/NoTailCalls.h:42 > +// from within a given function. For example, if you don't want foo nit: you could say scope instead of function. here. >> Source/WTF/wtf/NoTailCalls.h:55 >> +#define NO_TAIL_CALLS() WTF::NoTailCalls _noTailCalls_ > > An alternate name could be: > DONT_TAIL_CALL I think NO_TAIL_CALLS makes more sense. DONT_TAIL_CALL implies to me that the function can't be tail called, which is the opposite of what this does.
Comment on attachment 320847 [details] patch Nvm, it looks like this isn't working.
Created attachment 320858 [details] patch
Created attachment 320859 [details] patch
Comment on attachment 320859 [details] patch r=me if tests pass.
Comment on attachment 320859 [details] patch Clearing flags on attachment: 320859 Committed r222071: <http://trac.webkit.org/changeset/222071>
All reviewed patches have been landed. Closing bug.
<rdar://problem/34694446>