> On 13 May 2019, at 20:10, Jakub Jelinek <ja...@redhat.com> wrote: > > On Mon, May 13, 2019 at 12:52:54PM -0600, Jeff Law wrote: >> On 5/10/19 9:17 AM, Jakub Jelinek wrote: >>> On Fri, May 10, 2019 at 01:46:07PM +0000, Wilco Dijkstra wrote: >>>> And looking at the generated code, emitting a tailcall for this case is >>>> actually a >>>> de-optimization since the large eh_return epilog must be repeated for every >>>> tailcall. >>> >>> On x86_64, the code is shorter with the tail call rather than without. >>> >>> That said, here is actually tested workaround until targets are fixed. >>> Richard or Jeff, do we want this workaround? >>> >>> I don't see why we would need extra testsuite coverage for this, given the >>> number of FAILs or bootstrap issues on targets that are broken. >>> >>> 2019-05-10 Jakub Jelinek <ja...@redhat.com> >>> >>> PR c++/59813 >>> * unwind.inc (_Unwind_Resume_or_Rethrow): Add optimize attribute >>> to temporarily avoid tail calls in the function. >> I think my worry would be all the targets that support tail calls, but >> which we can't easily test. ie, we put in the hack, but when do we >> know it can be removed? >> >> Is there any property of the code that we can look at instead of a hack >> like this? > > We can test crtl->calls_eh_return if if make sure it is computed early > (before expansion), or we could just compute it in the ok_for_sibcall > hook if we add caching of that per function (check if any bb that doesn't > have successors and is still in GIMPLE IL ends with __builtin_eh_return > call), if not already crtl->calls_eh_return. > > I think only targets that define EH_RETURN_STACKADJ_RTX are problematic, and > from those only those that do not use eh_return_internal patterns (e.g. i386 > or bfin or riscv have special eh_return_internal patterns that expand > epilogue with not just true/false for sibcall/for_sibcall, but either a 3 > state argument or two bools whether it is sibcall/normal/eh_return). > Out of these, aarch64 and rs6000 have been fixed (though, the latter maybe > not for powerpc-darwin yet).
no, it’s quite a lot more involved to fix that - I would favour the solution above - just punting when calls_eh_return is true in rs6000_ok_for_sibcall, until there’s a chance to try and remove (or improve) the save_world code. At least that’s also a simple immediate fix for any other affected target. Iain > alpha, arc, cr16, csky, frv, m32c, m68k, mmix, msp430, nds32, > nios2, or1k, pa, sh, tilegx, tilepro, visium, xtensa > might be still broken. > > Some of those might be even harder to fix (e.g. alpha, pa, mmix to name a > few), because they don't even bother to pass the sibcall/for_sibcall > argument to epilogue expansion. > > Jakub
