| Message ID | 10247773.6SAYMWXB7c@polaris |
|---|---|
| State | New |
| Headers | show |
On Tue, Dec 6, 2016 at 9:52 AM, Eric Botcazou <ebotcazou@adacore.com> wrote: >> There are a couple of changes to the RTL expander for calls; they are >> supposed to be transparent but they might have tripped on a latent issue. > > Tentative fix attached, I need to test it extensively in Ada though. Thanks. Lynn, can you see if this patch fixes the bugs you see? > Btw, Ian, if the heap trampoline support is no longer used by the Go compiler, > you might want to remove it from the middle-end. Yes, I suppose so. The Go frontend hasn't used them for a while. Ian
I tried this patch applied against latest and it fixed the testcases that I had reported as failing, but the patch also causes libgo reflect testcase to fail. Still testing to verify and will report the failure details. On 12/06/2016 02:18 PM, Ian Lance Taylor wrote: > On Tue, Dec 6, 2016 at 9:52 AM, Eric Botcazou <ebotcazou@adacore.com> wrote: >>> There are a couple of changes to the RTL expander for calls; they are >>> supposed to be transparent but they might have tripped on a latent issue. >> Tentative fix attached, I need to test it extensively in Ada though. > Thanks. Lynn, can you see if this patch fixes the bugs you see? > >> Btw, Ian, if the heap trampoline support is no longer used by the Go compiler, >> you might want to remove it from the middle-end. > Yes, I suppose so. The Go frontend hasn't used them for a while. > > Ian > >
> I tried this patch applied against latest and it fixed the testcases > that I had reported as failing, but the patch also causes > > libgo reflect testcase to fail. Still testing to verify and will report > the failure details. Please double check, as I can reproduce neither on PowerPC64 nor on x86-64. In any case, the patch just reverts a problematic change so can do no harm. -- Eric Botcazou
> > Btw, Ian, if the heap trampoline support is no longer used by the Go > > compiler, you might want to remove it from the middle-end. > > Yes, I suppose so. The Go frontend hasn't used them for a while. Speaking of obsolete stuff, do you have any opinion on the below patch? https://gcc.gnu.org/ml/gcc-patches/2016-10/msg01586.html -- Eric Botcazou
Yes I rebuilt everything and now it all looks good. The previously failing testcases are now passing and no new regressions. I must have had something set incorrectly in my environment on my first try. Thanks! On 12/06/2016 04:26 PM, Eric Botcazou wrote: >> I tried this patch applied against latest and it fixed the testcases >> that I had reported as failing, but the patch also causes >> >> libgo reflect testcase to fail. Still testing to verify and will report >> the failure details. > Please double check, as I can reproduce neither on PowerPC64 nor on x86-64. > > In any case, the patch just reverts a problematic change so can do no harm. >
> Yes I rebuilt everything and now it all looks good. The previously > failing testcases are now passing and no new regressions. I must have had > something set incorrectly in my environment on my first try. Thanks for confirming. -- Eric Botcazou
Index: calls.c =================================================================== --- calls.c (revision 243245) +++ calls.c (working copy) @@ -3427,13 +3427,6 @@ expand_call (tree exp, rtx target, int i if (STRICT_ALIGNMENT) store_unaligned_arguments_into_pseudos (args, num_actuals); - /* Prepare the address of the call. This must be done before any - register parameters is loaded for find_first_parameter_load to - work properly in the presence of descriptors. */ - funexp = prepare_call_address (fndecl ? fndecl : fntype, funexp, - static_chain_value, &call_fusage, - reg_parm_seen, flags); - /* Now store any partially-in-registers parm. This is the last place a block-move can happen. */ if (reg_parm_seen) @@ -3544,6 +3537,9 @@ expand_call (tree exp, rtx target, int i } after_args = get_last_insn (); + funexp = prepare_call_address (fndecl ? fndecl : fntype, funexp, + static_chain_value, &call_fusage, + reg_parm_seen, flags); load_register_parameters (args, num_actuals, &call_fusage, flags, pass == 0, &sibcall_failure);