[arm] Backport -- Fix ICE during thunk generation with -mlong-calls

Message ID 23fcdf27-a6ee-79b1-4e5a-21ef0782ef99@foss.arm.com
State New
Headers show
Series
  • [arm] Backport -- Fix ICE during thunk generation with -mlong-calls
Related show

Commit Message

Mihail Ionescu Nov. 7, 2018, 5:49 p.m.
Hi All,

This is a backport from trunk for GCC 8 and 7.

SVN revision: r264595.

Regression tested on arm-none-eabi.


gcc/ChangeLog

2018-11-02  Mihail Ionescu  <mihail.ionescu@arm.com>

	Backport from mainiline
	2018-09-26  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.

gcc/testsuite/ChangeLog

2018-11-02  Mihail Ionescu  <mihail.ionescu@arm.com>

	Backport from mainiline
         2018-09-17  Eric Botcazou  <ebotcazou@adacore.com>

	* g++.dg/other/thunk2a.C: New test.
	* g++.dg/other/thunk2b.C: Likewise.


If everything is ok, could someone commit it on my behalf?

Best regards,
    Mihail

Comments

Ramana Radhakrishnan Nov. 8, 2018, 10:02 a.m. | #1
On 07/11/2018 17:49, Mihail Ionescu wrote:
> Hi All,
> 
> This is a backport from trunk for GCC 8 and 7.
> 
> SVN revision: r264595.
> 
> Regression tested on arm-none-eabi.
> 
> 
> gcc/ChangeLog
> 
> 2018-11-02  Mihail Ionescu  <mihail.ionescu@arm.com>
> 
> 	Backport from mainiline
> 	2018-09-26  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.
> 
> gcc/testsuite/ChangeLog
> 
> 2018-11-02  Mihail Ionescu  <mihail.ionescu@arm.com>
> 
> 	Backport from mainiline
>           2018-09-17  Eric Botcazou  <ebotcazou@adacore.com>
> 
> 	* g++.dg/other/thunk2a.C: New test.
> 	* g++.dg/other/thunk2b.C: Likewise.
> 
> 
> If everything is ok, could someone commit it on my behalf?
> 
> Best regards,
>      Mihail
> 

It is a regression since my rewrite of this code.

Ok to backport to the release branches, it's been on trunk for a while 
and not shown any issues - please give the release managers a day or so 
to object.

regards
Ramana
Sudakshina Das Nov. 8, 2018, 6:53 p.m. | #2
Hi Mihail

On 08/11/18 10:02, Ramana Radhakrishnan wrote:
> On 07/11/2018 17:49, Mihail Ionescu wrote:
>> Hi All,
>>
>> This is a backport from trunk for GCC 8 and 7.
>>
>> SVN revision: r264595.
>>
>> Regression tested on arm-none-eabi.
>>
>>
>> gcc/ChangeLog
>>
>> 2018-11-02  Mihail Ionescu  <mihail.ionescu@arm.com>
>>
>> 	Backport from mainiline
>> 	2018-09-26  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.
>>
>> gcc/testsuite/ChangeLog
>>
>> 2018-11-02  Mihail Ionescu  <mihail.ionescu@arm.com>
>>
>> 	Backport from mainiline
>>            2018-09-17  Eric Botcazou  <ebotcazou@adacore.com>
>>
>> 	* g++.dg/other/thunk2a.C: New test.
>> 	* g++.dg/other/thunk2b.C: Likewise.
>>
>>
>> If everything is ok, could someone commit it on my behalf?
>>
>> Best regards,
>>       Mihail
>>
> 
> It is a regression since my rewrite of this code.
> 
> Ok to backport to the release branches, it's been on trunk for a while
> and not shown any issues - please give the release managers a day or so
> to object.
> 
> regards
> Ramana
> 

Does this fix PR87867 you reported? If yes, then it would be easier
to add the PR tag in the ChangeLog so that the ticket gets updated once
committed.

Thanks
Sudi
Mihail Ionescu Nov. 9, 2018, 1:29 p.m. | #3
Hi Sudi

On 11/08/2018 06:53 PM, Sudakshina Das wrote:
> Hi Mihail
> 
> On 08/11/18 10:02, Ramana Radhakrishnan wrote:
>> On 07/11/2018 17:49, Mihail Ionescu wrote:
>>> Hi All,
>>>
>>> This is a backport from trunk for GCC 8 and 7.
>>>
>>> SVN revision: r264595.
>>>
>>> Regression tested on arm-none-eabi.
>>>
>>>
>>> gcc/ChangeLog
>>>
>>> 2018-11-02  Mihail Ionescu  <mihail.ionescu@arm.com>
>>>
>>> 	Backport from mainiline
>>> 	2018-09-26  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.
>>>
>>> gcc/testsuite/ChangeLog
>>>
>>> 2018-11-02  Mihail Ionescu  <mihail.ionescu@arm.com>
>>>
>>> 	Backport from mainiline
>>>             2018-09-17  Eric Botcazou  <ebotcazou@adacore.com>
>>>
>>> 	* g++.dg/other/thunk2a.C: New test.
>>> 	* g++.dg/other/thunk2b.C: Likewise.
>>>
>>>
>>> If everything is ok, could someone commit it on my behalf?
>>>
>>> Best regards,
>>>        Mihail
>>>
>>
>> It is a regression since my rewrite of this code.
>>
>> Ok to backport to the release branches, it's been on trunk for a while
>> and not shown any issues - please give the release managers a day or so
>> to object.
>>
>> regards
>> Ramana
>>
> 
> Does this fix PR87867 you reported? If yes, then it would be easier
> to add the PR tag in the ChangeLog so that the ticket gets updated once
> committed.
> 
> Thanks
> Sudi
> 

I've seen the patch has been commited (Ramana added the PR tag), but I
now know it has to be added and I will do it in the future.

Regards
Mihail

Patch

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 2ece668219f3ca34883cd882431b0a3c390d4d3c..c68311e0fa192c350d03eb2dd37eca92ae7b3cfa 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -17663,7 +17663,11 @@  arm_reorg (void)
 
   if (use_cmse)
     cmse_nonsecure_call_clear_caller_saved ();
-  if (TARGET_THUMB1)
+
+  /* We cannot run the Thumb passes for thunks because there is no CFG.  */
+  if (cfun->is_thunk)
+    ;
+  else if (TARGET_THUMB1)
     thumb1_reorg ();
   else if (TARGET_THUMB2)
     thumb2_reorg ();
@@ -26737,6 +26741,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.
   */
@@ -26774,9 +26780,22 @@  arm32_output_mi_thunk (FILE *file, tree, HOST_WIDE_INT delta,
       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);
diff --git a/gcc/testsuite/g++.dg/other/thunk2a.C b/gcc/testsuite/g++.dg/other/thunk2a.C
new file mode 100644
index 0000000000000000000000000000000000000000..8e5ebd4960df758fa77ff08b019e104870f36b45
--- /dev/null
+++ b/gcc/testsuite/g++.dg/other/thunk2a.C
@@ -0,0 +1,15 @@ 
+// { 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() {}
diff --git a/gcc/testsuite/g++.dg/other/thunk2b.C b/gcc/testsuite/g++.dg/other/thunk2b.C
new file mode 100644
index 0000000000000000000000000000000000000000..c8f4570923d8bde71547dd343de45edc0efeb2c7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/other/thunk2b.C
@@ -0,0 +1,16 @@ 
+// { dg-do compile { target arm*-*-* } }
+// { dg-options "-mlong-calls -ffunction-sections" }
+// { dg-additional-options "-fPIC" { target fpic } }
+
+class a {
+public:
+  virtual ~a();
+};
+
+class b : virtual a {};
+
+class c : b {
+  ~c();
+};
+
+c::~c() {}
diff --git a/gcc/testsuite/g++.dg/other/vthunk1.C b/gcc/testsuite/g++.dg/other/thunk1.C
similarity index 100%
rename from gcc/testsuite/g++.dg/other/vthunk1.C
rename to gcc/testsuite/g++.dg/other/thunk1.C