* [PATCH] Regression fix for PR target/61223
@ 2014-05-20 16:05 Alexey Merzlyakov
2014-05-20 16:24 ` Ramana Radhakrishnan
0 siblings, 1 reply; 4+ messages in thread
From: Alexey Merzlyakov @ 2014-05-20 16:05 UTC (permalink / raw)
To: gcc-patches
Cc: Richard Earnshaw, Ramana Radhakrishnan, sandra, terry.guo,
Yury Gribov, Viacheslav Garbuzov
[-- 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"
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Regression fix for PR target/61223
2014-05-20 16:05 [PATCH] Regression fix for PR target/61223 Alexey Merzlyakov
@ 2014-05-20 16:24 ` Ramana Radhakrishnan
2014-05-20 17:26 ` Yury Gribov
2014-05-22 13:29 ` Alexey Merzlyakov
0 siblings, 2 replies; 4+ messages in thread
From: Ramana Radhakrishnan @ 2014-05-20 16:24 UTC (permalink / raw)
To: Alexey Merzlyakov
Cc: gcc-patches, Richard Earnshaw, sandra, Terry Guo, Yury Gribov,
Viacheslav Garbuzov
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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Regression fix for PR target/61223
2014-05-20 16:24 ` Ramana Radhakrishnan
@ 2014-05-20 17:26 ` Yury Gribov
2014-05-22 13:29 ` Alexey Merzlyakov
1 sibling, 0 replies; 4+ messages in thread
From: Yury Gribov @ 2014-05-20 17:26 UTC (permalink / raw)
To: Ramana Radhakrishnan, Alexey Merzlyakov
Cc: gcc-patches, Richard Earnshaw, sandra, Terry Guo, Viacheslav Garbuzov
> For now, please revert your original patch
Alex asked me to revert patch for him. Done in r210650.
-Y
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Regression fix for PR target/61223
2014-05-20 16:24 ` Ramana Radhakrishnan
2014-05-20 17:26 ` Yury Gribov
@ 2014-05-22 13:29 ` Alexey Merzlyakov
1 sibling, 0 replies; 4+ messages in thread
From: Alexey Merzlyakov @ 2014-05-22 13:29 UTC (permalink / raw)
To: Ramana Radhakrishnan
Cc: gcc-patches, Richard Earnshaw, sandra, Terry Guo, Yury Gribov,
Viacheslav Garbuzov
[-- 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-05-22 13:29 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-20 16:05 [PATCH] Regression fix for PR target/61223 Alexey Merzlyakov
2014-05-20 16:24 ` Ramana Radhakrishnan
2014-05-20 17:26 ` Yury Gribov
2014-05-22 13:29 ` Alexey Merzlyakov
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).