* [ARM] Fix ICE during thunk generation with -mlong-calls
@ 2018-09-17 11:23 Eric Botcazou
2018-09-17 13:14 ` Richard Earnshaw (lists)
0 siblings, 1 reply; 6+ messages in thread
From: Eric Botcazou @ 2018-09-17 11:23 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1269 bytes --]
Hi,
this is a regression present on mainline, 8 and 7 branches. The new, RTL
implementation of arm32_output_mi_thunk breaks during the libstdc++ build if
you configure the compiler with -mlong-calls by default:
0xdb57eb gen_reg_rtx(machine_mode)
/home/eric/svn/gcc/gcc/emit-rtl.c:1155
0xde9ae7 force_reg(machine_mode, rtx_def*)
/home/eric/svn/gcc/gcc/explow.c:654
0x1bf73bf gen_sibcall(rtx_def*, rtx_def*, rtx_def*)
/home/eric/svn/gcc/gcc/config/arm/arm.md:8272
0x187d3b1 arm32_output_mi_thunk
/home/eric/svn/gcc/gcc/config/arm/arm.c:26762
0x187d4af arm_output_mi_thunk
/home/eric/svn/gcc/gcc/config/arm/arm.c:26783
0xcb9c94 cgraph_node::expand_thunk(bool, bool)
/home/eric/svn/gcc/gcc/cgraphunit.c:1783
because the code is wired for a short call. Moreover, in PIC mode you need to
work harder and fix up the minipool too with -mlong-calls.
Tested on ARM/Linux, OK for mainline, 8 and 7 branches?
2018-09-17 Eric Botcazou <ebotcazou@adacore.com>
* config/arm/arm.c (arm_reorg): Skip Thumb reorg pass for thunks.
(arm32_output_mi_thunk): Deal with long calls.
2018-09-17 Eric Botcazou <ebotcazou@adacore.com>
* g++.dg/other/thunk2a.C: New test.
* g++.dg/other/thunk2b.C: Likewise.
--
Eric Botcazou
[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 1468 bytes --]
Index: config/arm/arm.c
===================================================================
--- config/arm/arm.c (revision 264342)
+++ config/arm/arm.c (working copy)
@@ -17647,7 +17647,9 @@ arm_reorg (void)
if (use_cmse)
cmse_nonsecure_call_clear_caller_saved ();
- if (TARGET_THUMB1)
+ if (cfun->is_thunk)
+ ;
+ else if (TARGET_THUMB1)
thumb1_reorg ();
else if (TARGET_THUMB2)
thumb2_reorg ();
@@ -26721,6 +26723,8 @@ static void
arm32_output_mi_thunk (FILE *file, tree, HOST_WIDE_INT delta,
HOST_WIDE_INT vcall_offset, tree function)
{
+ const bool long_call_p = arm_is_long_call_p (function);
+
/* On ARM, this_regno is R0 or R1 depending on
whether the function returns an aggregate or not.
*/
@@ -26758,9 +26762,22 @@ arm32_output_mi_thunk (FILE *file, tree,
TREE_USED (function) = 1;
}
rtx funexp = XEXP (DECL_RTL (function), 0);
+ if (long_call_p)
+ {
+ emit_move_insn (temp, funexp);
+ funexp = temp;
+ }
funexp = gen_rtx_MEM (FUNCTION_MODE, funexp);
- rtx_insn * insn = emit_call_insn (gen_sibcall (funexp, const0_rtx, NULL_RTX));
+ rtx_insn *insn = emit_call_insn (gen_sibcall (funexp, const0_rtx, NULL_RTX));
SIBLING_CALL_P (insn) = 1;
+ emit_barrier ();
+
+ /* Indirect calls require a bit of fixup in PIC mode. */
+ if (long_call_p)
+ {
+ split_all_insns_noflow ();
+ arm_reorg ();
+ }
insn = get_insns ();
shorten_branches (insn);
[-- Attachment #3: thunk2a.C --]
[-- Type: text/x-c++src, Size: 194 bytes --]
// { dg-do compile { target arm*-*-* } }
// { dg-options "-mlong-calls -ffunction-sections }
class a {
public:
virtual ~a();
};
class b : virtual a {};
class c : b {
~c();
};
c::~c() {}
[-- Attachment #4: thunk2b.C --]
[-- Type: text/x-c++src, Size: 208 bytes --]
// { dg-do compile { target arm*-*-* && fpic } }
// { dg-options "-mlong-calls -ffunction-sections -fPIC }
class a {
public:
virtual ~a();
};
class b : virtual a {};
class c : b {
~c();
};
c::~c() {}
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [ARM] Fix ICE during thunk generation with -mlong-calls
2018-09-17 11:23 [ARM] Fix ICE during thunk generation with -mlong-calls Eric Botcazou
@ 2018-09-17 13:14 ` Richard Earnshaw (lists)
2018-09-18 9:18 ` Eric Botcazou
0 siblings, 1 reply; 6+ messages in thread
From: Richard Earnshaw (lists) @ 2018-09-17 13:14 UTC (permalink / raw)
To: Eric Botcazou, gcc-patches
On 17/09/18 12:19, Eric Botcazou wrote:
> Hi,
>
> this is a regression present on mainline, 8 and 7 branches. The new, RTL
> implementation of arm32_output_mi_thunk breaks during the libstdc++ build if
> you configure the compiler with -mlong-calls by default:
>
> 0xdb57eb gen_reg_rtx(machine_mode)
> /home/eric/svn/gcc/gcc/emit-rtl.c:1155
> 0xde9ae7 force_reg(machine_mode, rtx_def*)
> /home/eric/svn/gcc/gcc/explow.c:654
> 0x1bf73bf gen_sibcall(rtx_def*, rtx_def*, rtx_def*)
> /home/eric/svn/gcc/gcc/config/arm/arm.md:8272
> 0x187d3b1 arm32_output_mi_thunk
> /home/eric/svn/gcc/gcc/config/arm/arm.c:26762
> 0x187d4af arm_output_mi_thunk
> /home/eric/svn/gcc/gcc/config/arm/arm.c:26783
> 0xcb9c94 cgraph_node::expand_thunk(bool, bool)
> /home/eric/svn/gcc/gcc/cgraphunit.c:1783
>
> because the code is wired for a short call. Moreover, in PIC mode you need to
> work harder and fix up the minipool too with -mlong-calls.
>
> Tested on ARM/Linux, OK for mainline, 8 and 7 branches?
>
>
> 2018-09-17 Eric Botcazou <ebotcazou@adacore.com>
>
> * config/arm/arm.c (arm_reorg): Skip Thumb reorg pass for thunks.
this seems to contradict your statement above about having to work
harder to fix up minipools. why is it correct to skip this entirely?
> (arm32_output_mi_thunk): Deal with long calls.
>
>
> 2018-09-17 Eric Botcazou <ebotcazou@adacore.com>
>
> * g++.dg/other/thunk2a.C: New test.
> * g++.dg/other/thunk2b.C: Likewise.
>
>
> p.diff
>
>
> Index: config/arm/arm.c
> ===================================================================
> --- config/arm/arm.c (revision 264342)
> +++ config/arm/arm.c (working copy)
> @@ -17647,7 +17647,9 @@ arm_reorg (void)
>
> if (use_cmse)
> cmse_nonsecure_call_clear_caller_saved ();
> - if (TARGET_THUMB1)
> + if (cfun->is_thunk)
> + ;
> + else if (TARGET_THUMB1)
> thumb1_reorg ();
> else if (TARGET_THUMB2)
> thumb2_reorg ();
> @@ -26721,6 +26723,8 @@ static void
> arm32_output_mi_thunk (FILE *file, tree, HOST_WIDE_INT delta,
> HOST_WIDE_INT vcall_offset, tree function)
> {
> + const bool long_call_p = arm_is_long_call_p (function);
> +
> /* On ARM, this_regno is R0 or R1 depending on
> whether the function returns an aggregate or not.
> */
> @@ -26758,9 +26762,22 @@ arm32_output_mi_thunk (FILE *file, tree,
> TREE_USED (function) = 1;
> }
> rtx funexp = XEXP (DECL_RTL (function), 0);
> + if (long_call_p)
> + {
> + emit_move_insn (temp, funexp);
> + funexp = temp;
> + }
> funexp = gen_rtx_MEM (FUNCTION_MODE, funexp);
> - rtx_insn * insn = emit_call_insn (gen_sibcall (funexp, const0_rtx, NULL_RTX));
> + rtx_insn *insn = emit_call_insn (gen_sibcall (funexp, const0_rtx, NULL_RTX));
> SIBLING_CALL_P (insn) = 1;
> + emit_barrier ();
Why do we need a barrier here unconditionally (ie in the non-longcall case)?
R.
> +
> + /* Indirect calls require a bit of fixup in PIC mode. */
> + if (long_call_p)
> + {
> + split_all_insns_noflow ();
> + arm_reorg ();
> + }
>
> insn = get_insns ();
> shorten_branches (insn);
>
>
> thunk2a.C
>
>
> // { dg-do compile { target arm*-*-* } }
> // { dg-options "-mlong-calls -ffunction-sections }
>
> class a {
> public:
> virtual ~a();
> };
>
> class b : virtual a {};
>
> class c : b {
> ~c();
> };
>
> c::~c() {}
>
>
> thunk2b.C
>
>
> // { dg-do compile { target arm*-*-* && fpic } }
> // { dg-options "-mlong-calls -ffunction-sections -fPIC }
>
> class a {
> public:
> virtual ~a();
> };
>
> class b : virtual a {};
>
> class c : b {
> ~c();
> };
>
> c::~c() {}
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [ARM] Fix ICE during thunk generation with -mlong-calls
2018-09-17 13:14 ` Richard Earnshaw (lists)
@ 2018-09-18 9:18 ` Eric Botcazou
2018-09-18 10:51 ` Richard Earnshaw (lists)
0 siblings, 1 reply; 6+ messages in thread
From: Eric Botcazou @ 2018-09-18 9:18 UTC (permalink / raw)
To: Richard Earnshaw (lists); +Cc: gcc-patches
> this seems to contradict your statement above about having to work
> harder to fix up minipools.
Why? Fixing up minipools is done in the generic ARM reorg pass, not in the
Thumb reorg pass(es).
> Why do we need a barrier here unconditionally (ie in the non-longcall case)?
We don't, but it doesn't harm to put it either. For example, the x86, PowerPC
and SPARC ports always do it.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [ARM] Fix ICE during thunk generation with -mlong-calls
2018-09-18 9:18 ` Eric Botcazou
@ 2018-09-18 10:51 ` Richard Earnshaw (lists)
2018-09-24 9:26 ` Eric Botcazou
0 siblings, 1 reply; 6+ messages in thread
From: Richard Earnshaw (lists) @ 2018-09-18 10:51 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches
On 18/09/18 10:00, Eric Botcazou wrote:
>> this seems to contradict your statement above about having to work
>> harder to fix up minipools.
>
> Why? Fixing up minipools is done in the generic ARM reorg pass, not in the
> Thumb reorg pass(es).
>
Ah! But that still doesn't explain why you want to skip these passes
when building thunks.
>> Why do we need a barrier here unconditionally (ie in the non-longcall case)?
>
> We don't, but it doesn't harm to put it either. For example, the x86, PowerPC
> and SPARC ports always do it.
>
So is the barrier correct, or isn't it? There's really no two ways
about this. I don't like arbitrary changes that are justified solely on
'that's what another port does'.
R.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [ARM] Fix ICE during thunk generation with -mlong-calls
2018-09-18 10:51 ` Richard Earnshaw (lists)
@ 2018-09-24 9:26 ` Eric Botcazou
2018-09-25 14:31 ` Richard Earnshaw (lists)
0 siblings, 1 reply; 6+ messages in thread
From: Eric Botcazou @ 2018-09-24 9:26 UTC (permalink / raw)
To: Richard Earnshaw (lists); +Cc: gcc-patches
> Ah! But that still doesn't explain why you want to skip these passes
> when building thunks.
They simply don't work because there is no CFG for thunks; I can add a blurb
about that.
> So is the barrier correct, or isn't it? There's really no two ways
> about this. I don't like arbitrary changes that are justified solely on
> 'that's what another port does'.
The barrier is required by the arm_reorg pass, but it is optional when the
pass is not run. I think that we can consider that it is also correct.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [ARM] Fix ICE during thunk generation with -mlong-calls
2018-09-24 9:26 ` Eric Botcazou
@ 2018-09-25 14:31 ` Richard Earnshaw (lists)
0 siblings, 0 replies; 6+ messages in thread
From: Richard Earnshaw (lists) @ 2018-09-25 14:31 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches
On 24/09/18 10:19, Eric Botcazou wrote:
>> Ah! But that still doesn't explain why you want to skip these passes
>> when building thunks.
>
> They simply don't work because there is no CFG for thunks; I can add a blurb
> about that.
Yes, this needs a comment as it's far from obvious when looking at the code.
>
>> So is the barrier correct, or isn't it? There's really no two ways
>> about this. I don't like arbitrary changes that are justified solely on
>> 'that's what another port does'.
>
> The barrier is required by the arm_reorg pass, but it is optional when the
> pass is not run. I think that we can consider that it is also correct.
>
Ah, because you're now calling arm_reorg directly and it needs the
barrier to drop the minipool in the right place.
So OK with the additional comment.
R.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-09-25 14:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-17 11:23 [ARM] Fix ICE during thunk generation with -mlong-calls Eric Botcazou
2018-09-17 13:14 ` Richard Earnshaw (lists)
2018-09-18 9:18 ` Eric Botcazou
2018-09-18 10:51 ` Richard Earnshaw (lists)
2018-09-24 9:26 ` Eric Botcazou
2018-09-25 14:31 ` Richard Earnshaw (lists)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).