[Bug bootstrap/90418] [10 Regression] powerpc-darwin9 bootstrap fails after r271013
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90418 Jakub Jelinek changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #15 from Jakub Jelinek --- Should be fixed now.
[Bug bootstrap/90418] [10 Regression] powerpc-darwin9 bootstrap fails after r271013
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90418 --- Comment #14 from Jakub Jelinek --- Author: jakub Date: Mon May 20 21:33:46 2019 New Revision: 271440 URL: https://gcc.gnu.org/viewcvs?rev=271440=gcc=rev Log: PR c++/59813 PR target/90418 * function.h (struct function): Add calls_eh_return member. * gimplify.c (gimplify_call_expr): Set cfun->calls_eh_return when gimplifying __builtin_eh_return call. * tree-inline.c (initialize_cfun): Copy calls_eh_return from src_cfun to cfun. (expand_call_inline): Or in src_cfun->calls_eh_return into dst_cfun->calls_eh_return. * tree-tailcall.c (suitable_for_tail_call_opt_p): Return false if cfun->calls_eh_return. * lto-streamer-in.c (input_struct_function_base): Read calls_eh_return. * lto-streamer-out.c (output_struct_function_base): Write calls_eh_return. Modified: trunk/gcc/ChangeLog trunk/gcc/function.h trunk/gcc/gimplify.c trunk/gcc/lto-streamer-in.c trunk/gcc/lto-streamer-out.c trunk/gcc/tree-inline.c trunk/gcc/tree-tailcall.c
[Bug bootstrap/90418] [10 Regression] powerpc-darwin9 bootstrap fails after r271013
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90418 --- Comment #13 from David Edelsohn --- Author: dje Date: Mon May 13 15:19:50 2019 New Revision: 271130 URL: https://gcc.gnu.org/viewcvs?rev=271130=gcc=rev Log: PR target/90418 * config/rs6000/rs6000.c (rs6000_emit_epilogue): Don't load EH data registers in sibcall epilogues. Don't add EH_RETURN_STACKADJ_RTX to sp in sibcall epilogues. Modified: trunk/gcc/ChangeLog trunk/gcc/config/rs6000/rs6000.c
[Bug bootstrap/90418] [10 Regression] powerpc-darwin9 bootstrap fails after r271013
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90418 --- Comment #12 from Iain Sandoe --- (In reply to David Edelsohn from comment #11) > Created attachment 46345 [details] > AIX EH sibcall patch > > This patch seems to fix the EH failures on AIX. > > Because of Darwin use of save_world, Darwin may need the more invasive > change to disable sibcalls. yes, that's what I'm expecting (so long as Jakub's patch is applied to make calls_eh_return valid at the point of checking - the two-liner I posted will work)
[Bug bootstrap/90418] [10 Regression] powerpc-darwin9 bootstrap fails after r271013
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90418 --- Comment #11 from David Edelsohn --- Created attachment 46345 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46345=edit AIX EH sibcall patch This patch seems to fix the EH failures on AIX. Because of Darwin use of save_world, Darwin may need the more invasive change to disable sibcalls.
[Bug bootstrap/90418] [10 Regression] powerpc-darwin9 bootstrap fails after r271013
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90418 --- Comment #10 from Iain Sandoe --- (In reply to David Edelsohn from comment #9) > Changing > > /* Extra stack adjustment for exception handler return. */ > if (crtl->calls_eh_return) > emit_insn (gen_addsi3 (stack_pointer_rtx, stack_pointer_rtx, >EH_RETURN_STACKADJ_RTX)); > > is not sufficient for AIX, at least. I am testing an additional change to > the earlier code to load exception handler data. I think it's a non-starter trying to fix Darwin's save_world stuff, so I'm going to stick with disabling tail calls in eh_return fns and then defer anything else until we find a way to remove the save_world stuff.
[Bug bootstrap/90418] [10 Regression] powerpc-darwin9 bootstrap fails after r271013
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90418 --- Comment #9 from David Edelsohn --- Changing /* Extra stack adjustment for exception handler return. */ if (crtl->calls_eh_return) emit_insn (gen_addsi3 (stack_pointer_rtx, stack_pointer_rtx, EH_RETURN_STACKADJ_RTX)); is not sufficient for AIX, at least. I am testing an additional change to the earlier code to load exception handler data.
[Bug bootstrap/90418] [10 Regression] powerpc-darwin9 bootstrap fails after r271013
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90418 --- Comment #8 from Segher Boessenkool --- It also fails all over on powerpc-linux. Pretty much all targets just do something like /* Extra stack adjustment for exception handler return. */ if (crtl->calls_eh_return) emit_insn (gen_addsi3 (stack_pointer_rtx, stack_pointer_rtx, EH_RETURN_STACKADJ_RTX)); /* Now we can return. */ emit_jump_insn (gen_simple_return ()); A fix should be target-independent, or it should fix all targets.
[Bug bootstrap/90418] [10 Regression] powerpc-darwin9 bootstrap fails after r271013
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90418 --- Comment #7 from Iain Sandoe --- Created attachment 46340 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46340=edit Patch under test This is a combination of Jakub's patch https://gcc.gnu.org/ml/gcc-patches/2019-05/msg00484.html + a fragment to note that WORLD_SAVE_P is set in -mdebug=stack + change to rs6000_function_ok_for_sibcall to return false for eh_return cases.
[Bug bootstrap/90418] [10 Regression] powerpc-darwin9 bootstrap fails after r271013
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90418 David Edelsohn changed: What|Removed |Added Target|powerpc-apple-darwin9 |powerpc-apple-darwin9 ||powerpc-ibm-aix* CC||dje at gcc dot gnu.org Host|powerpc-apple-darwin9 | Build|powerpc-apple-darwin9 | --- Comment #6 from David Edelsohn --- The patch also introduced a large number of EH testsuite failures for AIX.
[Bug bootstrap/90418] [10 Regression] powerpc-darwin9 bootstrap fails after r271013
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90418 --- Comment #5 from Iain Sandoe --- (In reply to Iain Sandoe from comment #4) > (In reply to Jakub Jelinek from comment #3) > > (In reply to Iain Sandoe from comment #2) > > > (In reply to Jakub Jelinek from comment #1) > > > Though, I don't understand why that > > if (DEFAULT_ABI == ABI_DARWIN && crtl->calls_eh_return) return false; > > wouldn't work in rs6000_function_ok_for_sibcall, because that conditional > > should make sure we don't have any tail calls in such functions and thus the > > above shouldn't trigger. > > Agreed, and that's exactly the code I have present, and I just did a clean > build. However, as you suspect the sibcall is still present. Perhaps > crtl->calls_eh_return is not somehow set or somehow a sib call is generated > without checking it's OK. Will look more later. Hm. So it seems that the problem is in expand - the call to rs6000_function_ok_for_sibcall() occurs when the tail call is encountered, but that is before the call to expand_eh_return() - which sets crtl->calls_eh_return. If I jam sibcalls off completely, of course, it works. Trying the posted patch for gcc/tree-tailcall.c
[Bug bootstrap/90418] [10 Regression] powerpc-darwin9 bootstrap fails after r271013
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90418
--- Comment #4 from Iain Sandoe ---
(In reply to Jakub Jelinek from comment #3)
> (In reply to Iain Sandoe from comment #2)
> > (In reply to Jakub Jelinek from comment #1)
> Though, I don't understand why that
> if (DEFAULT_ABI == ABI_DARWIN && crtl->calls_eh_return) return false;
> wouldn't work in rs6000_function_ok_for_sibcall, because that conditional
> should make sure we don't have any tail calls in such functions and thus the
> above shouldn't trigger.
Agreed, and that's exactly the code I have present, and I just did a clean
build. However, as you suspect the sibcall is still present. Perhaps
crtl->calls_eh_return is not somehow set or somehow a sib call is generated
without checking it's OK. Will look more later.
(call_insn/j 16 14 17 3 (parallel [
(set (reg:SI 3 r3)
(call (mem:SI (symbol_ref:SI ("_Unwind_RaiseException") [flags
0x403] ) [0
_Unwind_RaiseException S4 A8])
(const_int 32 [0x20])))
(use (const_int 0 [0]))
(simple_return)
]) "/src-local/gcc-git/libgcc/unwind.inc":264:12 682
{*sibcall_value_local32}
(expr_list:REG_CALL_DECL (symbol_ref:SI ("_Unwind_RaiseException") [flags
0x403] )
(nil))
(expr_list:SI (use (reg:SI 3 r3))
(nil)))
[Bug bootstrap/90418] [10 Regression] powerpc-darwin9 bootstrap fails after r271013
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90418
--- Comment #3 from Jakub Jelinek ---
(In reply to Iain Sandoe from comment #2)
> (In reply to Jakub Jelinek from comment #1)
> > Most likely similar problem to the one analyzed in PR59813, after all, it is
> > the same function. Previously, in that function there were no tail calls
> > and most likely no tailcalls in any function with __builtin_eh_return call,
> > now it is possible, so various targets need to either catch up and handle
> > that case properly, or declare they don't want to support tailcalls if
> > cfun->calls_eh_return in *_ok_for_sibcall target hook.
>
> thanks for the suggestion, I tried disabling sib calls when
> crtl->calls_eh_return (it is probably something that should be done anyway,
> given what's below). That's not sufficient to resolve the bug.
I guess the problem is then:
/* This will not work in conjunction with sibcalls. Make sure there
are none. (This check is expensive, but seldom executed.) */
if (WORLD_SAVE_P (info))
{
rtx_insn *insn;
for (insn = get_last_insn_anywhere (); insn; insn = PREV_INSN (insn))
if (CALL_P (insn) && SIBLING_CALL_P (insn))
{
info->world_save_p = 0;
break;
}
}
if powerpc-darwin relies on world save in order to implement
__builtin_eh_return. Though, I don't understand why that
if (DEFAULT_ABI == ABI_DARWIN && crtl->calls_eh_return) return false;
wouldn't work in rs6000_function_ok_for_sibcall, because that conditional
should make sure we don't have any tail calls in such functions and thus the
above shouldn't trigger.
> We actually generate correct code for -O1, but fail at -O2,3,s.
tail calls are optimized only at -O2.
[Bug bootstrap/90418] [10 Regression] powerpc-darwin9 bootstrap fails after r271013
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90418 --- Comment #2 from Iain Sandoe --- (In reply to Jakub Jelinek from comment #1) > Most likely similar problem to the one analyzed in PR59813, after all, it is > the same function. Previously, in that function there were no tail calls > and most likely no tailcalls in any function with __builtin_eh_return call, > now it is possible, so various targets need to either catch up and handle > that case properly, or declare they don't want to support tailcalls if > cfun->calls_eh_return in *_ok_for_sibcall target hook. thanks for the suggestion, I tried disabling sib calls when crtl->calls_eh_return (it is probably something that should be done anyway, given what's below). That's not sufficient to resolve the bug. We actually generate correct code for -O1, but fail at -O2,3,s. There seem to be two problems. powerpc-darwin has a facility for the unwinder that allows the same binary to run on a machine with or without altivec. The unwind save/restore is delegated to a function "save_world" that tests an OS bit to determine if the altivec regs should be saved/restored. 1) We are correctly calling this for <= O1 - We are not (for some TBD reason) calling it at greater optimisations. 2) however, it's not clear why the current generated code isn't lowerable (even if it would not work at runtime on a machine without altivec). longer-term there's an intention to remove the "save_world" machinery, but we have to replace the relevant routines in libgcc to do that. It seems that right now if we just switched it off, the codegen would fail anyway.
[Bug bootstrap/90418] [10 Regression] powerpc-darwin9 bootstrap fails after r271013
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90418 Richard Biener changed: What|Removed |Added Priority|P3 |P1
[Bug bootstrap/90418] [10 Regression] powerpc-darwin9 bootstrap fails after r271013
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90418 --- Comment #1 from Jakub Jelinek --- Most likely similar problem to the one analyzed in PR59813, after all, it is the same function. Previously, in that function there were no tail calls and most likely no tailcalls in any function with __builtin_eh_return call, now it is possible, so various targets need to either catch up and handle that case properly, or declare they don't want to support tailcalls if cfun->calls_eh_return in *_ok_for_sibcall target hook.
[Bug bootstrap/90418] [10 Regression] powerpc-darwin9 bootstrap fails after r271013
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90418 Iain Sandoe changed: What|Removed |Added Target||powerpc-apple-darwin9 Status|UNCONFIRMED |NEW Known to work||9.1.0 Keywords||ice-on-valid-code Last reconfirmed||2019-05-09 CC||jakub at gcc dot gnu.org Host||powerpc-apple-darwin9 Ever confirmed|0 |1 Target Milestone|--- |10.0 Known to fail||10.0 Build||powerpc-apple-darwin9
