[PATCH] Regression fix for PR target/61223

[PATCH] Regression fix for PR target/61223

[Alexey Merzlyakov]

[-- Attachment #1: Type: text/plain, Size: 581 bytes --]

Hi all,

This is a fix for thumb1 build fail on trunk appeared since 
rev.210515(pr60758).

On thumb1 targets the LR can not be used as argument of POP instruction.
To keep a support of __cxa_end_cleanup backtracing on thumb1
we can make additional register operations before push/pop.
But I guess, this will have a negative effect on __cxa_end_cleanup 
performance.
So, I propose to preserve __cxa_end_cleanup backtracing on thumb2 
architectures and revert it on thumb1.

Regtest was finished with no regressions on C/C++ arm-linux-gnueabi(sf).

Best regards,
Merzlyakov Alexey


[-- Attachment #2: cxa_fix_fix.patch --]
[-- Type: text/x-diff, Size: 1361 bytes --]

2014-05-20  Alexey Merzlyakov  <alexey.merzlyakov@samsung.com>

	PR target/61223
	* libsupc++/eh_arm.cc (__cxa_end_cleanup): Revert backtracing support
	on thumb1.

diff --git a/libstdc++-v3/libsupc++/eh_arm.cc b/libstdc++-v3/libsupc++/eh_arm.cc
index 6a45af5..8ad327d 100644
--- a/libstdc++-v3/libsupc++/eh_arm.cc
+++ b/libstdc++-v3/libsupc++/eh_arm.cc
@@ -200,7 +200,8 @@ asm (".global __cxa_end_cleanup\n"
 #else
 // Assembly wrapper to call __gnu_end_cleanup without clobbering r1-r3.
 // Also push lr to preserve stack alignment and to allow backtracing.
-#ifdef __thumb__
+// On thumb1 architectures push r4 because pop lr is not available.
+#ifdef __thumb2__
 asm ("  .pushsection .text.__cxa_end_cleanup\n"
 "	.global __cxa_end_cleanup\n"
 "	.type __cxa_end_cleanup, \"function\"\n"
@@ -214,6 +215,17 @@ asm ("  .pushsection .text.__cxa_end_cleanup\n"
 "	bl\t_Unwind_Resume @ Never returns\n"
 "	.fnend\n"
 "	.popsection\n");
+#elif defined(__thumb__)
+asm ("  .pushsection .text.__cxa_end_cleanup\n"
+"	.global __cxa_end_cleanup\n"
+"	.type __cxa_end_cleanup, \"function\"\n"
+"	.thumb_func\n"
+"__cxa_end_cleanup:\n"
+"	push\t{r1, r2, r3, r4}\n"
+"	bl\t__gnu_end_cleanup\n"
+"	pop\t{r1, r2, r3, r4}\n"
+"	bl\t_Unwind_Resume @ Never returns\n"
+"	.popsection\n");
 #else
 asm ("  .pushsection .text.__cxa_end_cleanup\n"
 "	.global __cxa_end_cleanup\n"

Re: [PATCH] Regression fix for PR target/61223

[Ramana Radhakrishnan]

On 05/20/14 17:05, Alexey Merzlyakov wrote:
> Hi all,
>
> This is a fix for thumb1 build fail on trunk appeared since
> rev.210515(pr60758).
>
> On thumb1 targets the LR can not be used as argument of POP instruction.
> To keep a support of __cxa_end_cleanup backtracing on thumb1
> we can make additional register operations before push/pop.
> But I guess, this will have a negative effect on __cxa_end_cleanup
> performance.
> So, I propose to preserve __cxa_end_cleanup backtracing on thumb2
> architectures and revert it on thumb1.
>
> Regtest was finished with no regressions on C/C++ arm-linux-gnueabi(sf).

For now, please revert your original patch and one of Richard or me will 
try to look at it in the morning. The thumb1 case is slightly more 
complicated than this.

I don't like this approach as you are introducing the problem again in 
ARM state and leaving the problem as is for Thumb1.

regards
Ramana

>
> Best regards,
> Merzlyakov Alexey
>

Re: [PATCH] Regression fix for PR target/61223

[Yury Gribov]

 > For now, please revert your original patch

Alex asked me to revert patch for him. Done in r210650.

-Y

Re: [PATCH] Regression fix for PR target/61223

[Alexey Merzlyakov]

[-- Attachment #1: Type: text/plain, Size: 860 bytes --]

On 20.05.2014 20:24, Ramana Radhakrishnan wrote:
> For now, please revert your original patch and one of Richard or me 
> will try to look at it in the morning. The thumb1 case is slightly 
> more complicated than this.
>
> I don't like this approach as you are introducing the problem again in 
> ARM state
I am not sure, could you provide more details: why do you think ARM 
state is invalid here?

> and leaving the problem as is for Thumb1.
True. I've came up a new draft implementation which supports also thumb1 
(untested). Does it look saner?

Actually, it seems "pop lr" is not really necessary because following by 
"bl _Unwind_Resume @ Never returns" instruction overwrites lr immediately.
Perhaps it may make sense to replace "bl _Unwind_Resume" with "b 
_Unwind_Resume" to keep backtracing also in _Unwind_Resume?

Best regards,
Merzlyakov Alexey

[-- Attachment #2: cxa_fix_fix2.patch --]
[-- Type: text/x-diff, Size: 1782 bytes --]

--- gcc-4_9-vanilla/libstdc++-v3/libsupc++/eh_arm.cc	2014-05-22 17:20:43.018209504 +0400
+++ gcc-4_9/libstdc++-v3/libsupc++/eh_arm.cc	2014-05-22 16:32:43.726271177 +0400
@@ -199,27 +199,51 @@
 "	nop		5\n");
 #else
 // Assembly wrapper to call __gnu_end_cleanup without clobbering r1-r3.
-// Also push r4 to preserve stack alignment.
-#ifdef __thumb__
+// Also push lr to preserve stack alignment and to allow backtracing.
+#ifdef __thumb2__
 asm ("  .pushsection .text.__cxa_end_cleanup\n"
 "	.global __cxa_end_cleanup\n"
 "	.type __cxa_end_cleanup, \"function\"\n"
 "	.thumb_func\n"
 "__cxa_end_cleanup:\n"
-"	push\t{r1, r2, r3, r4}\n"
+"	.fnstart\n"
+"	push\t{r1, r2, r3, lr}\n"
+"	.save\t{r1, r2, r3, lr}\n"
 "	bl\t__gnu_end_cleanup\n"
-"	pop\t{r1, r2, r3, r4}\n"
+"	pop\t{r1, r2, r3, lr}\n"
 "	bl\t_Unwind_Resume @ Never returns\n"
+"	.fnend\n"
+"	.popsection\n");
+#elif defined(__thumb__)
+asm ("  .pushsection .text.__cxa_end_cleanup\n"
+"	.global __cxa_end_cleanup\n"
+"	.type __cxa_end_cleanup, \"function\"\n"
+"	.thumb_func\n"
+"__cxa_end_cleanup:\n"
+"	.fnstart\n"
+"	push\t{r1, r2, r3}\n"
+"	.save\t{r1, r2, r3}\n"
+"	push\t{lr}\n"
+"	.save\t{lr}\n"
+"	bl\t__gnu_end_cleanup\n"
+"	pop\t{r3}\n"
+"	mov\tlr, r3\n"
+"	pop\t{r1, r2, r3}\n"
+"	bl\t_Unwind_Resume @ Never returns\n"
+"	.fnend\n"
 "	.popsection\n");
 #else
 asm ("  .pushsection .text.__cxa_end_cleanup\n"
 "	.global __cxa_end_cleanup\n"
 "	.type __cxa_end_cleanup, \"function\"\n"
 "__cxa_end_cleanup:\n"
-"	stmfd\tsp!, {r1, r2, r3, r4}\n"
+"	.fnstart\n"
+"	stmfd\tsp!, {r1, r2, r3, lr}\n"
+"	.save\t{r1, r2, r3, lr}\n"
 "	bl\t__gnu_end_cleanup\n"
-"	ldmfd\tsp!, {r1, r2, r3, r4}\n"
+"	ldmfd\tsp!, {r1, r2, r3, lr}\n"
 "	bl\t_Unwind_Resume @ Never returns\n"
+"	.fnend\n"
 "	.popsection\n");
 #endif
 #endif