* [PATCH, AArch64, Testsuite] Specify -fno-use-caller-save for func-ret* tests
@ 2014-07-01 15:51 Yufeng Zhang
2014-07-01 17:26 ` Jeff Law
0 siblings, 1 reply; 4+ messages in thread
From: Yufeng Zhang @ 2014-07-01 15:51 UTC (permalink / raw)
To: gcc-patches, Marcus Shawcroft
[-- Attachment #1: Type: text/plain, Size: 928 bytes --]
Hi,
This patch resolves a conflict between the aapcs64 test framework for
func-ret tests and the optimization option -fuse-caller-save, which was
enabled by default at -O1 or above recently.
Basically, the test framework has an inline-assembly based mechanism in
place which invokes the test facility function right on the return of a
tested function. The compiler with -fuse-caller-save is unable to
identify the unconventional call graph and carries out the optimization
regardless.
Adding explicit LR clobbering field to the inline assembly doesn't solve
the issue as the compiler would simply generate extra save/store of LR
in the prologue/epilogue.
OK for the trunk?
Thanks,
Yufeng
gcc/testsuite/
* gcc.target/aarch64/aapcs64/aapcs64.exp:
(additional_flags_for_func_ret): New variable based on $additional_flags
plus -fno-use-caller-save.
(func-ret-*.c): Use the new variable.
[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 890 bytes --]
diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/aapcs64.exp b/gcc/testsuite/gcc.target/aarch64/aapcs64/aapcs64.exp
index 195f977..fdfbff1 100644
--- a/gcc/testsuite/gcc.target/aarch64/aapcs64/aapcs64.exp
+++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/aapcs64.exp
@@ -48,11 +48,15 @@ foreach src [lsort [glob -nocomplain $srcdir/$subdir/va_arg-*.c]] {
}
# Test function return value.
+# Disable -fuse-caller-save to prevent the compiler from generating
+# conflicting code.
+set additional_flags_for_func_ret $additional_flags
+append additional_flags_for_func_ret " -fno-use-caller-save"
foreach src [lsort [glob -nocomplain $srcdir/$subdir/func-ret-*.c]] {
if {[runtest_file_p $runtests $src]} {
c-torture-execute [list $src \
$srcdir/$subdir/abitest.S] \
- $additional_flags
+ $additional_flags_for_func_ret
}
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH, AArch64, Testsuite] Specify -fno-use-caller-save for func-ret* tests
2014-07-01 15:51 [PATCH, AArch64, Testsuite] Specify -fno-use-caller-save for func-ret* tests Yufeng Zhang
@ 2014-07-01 17:26 ` Jeff Law
2014-07-08 19:45 ` Tom de Vries
0 siblings, 1 reply; 4+ messages in thread
From: Jeff Law @ 2014-07-01 17:26 UTC (permalink / raw)
To: Yufeng Zhang, gcc-patches, Marcus Shawcroft
On 07/01/14 09:51, Yufeng Zhang wrote:
> Hi,
>
> This patch resolves a conflict between the aapcs64 test framework for
> func-ret tests and the optimization option -fuse-caller-save, which was
> enabled by default at -O1 or above recently.
>
> Basically, the test framework has an inline-assembly based mechanism in
> place which invokes the test facility function right on the return of a
> tested function. The compiler with -fuse-caller-save is unable to
> identify the unconventional call graph and carries out the optimization
> regardless.
>
> Adding explicit LR clobbering field to the inline assembly doesn't solve
> the issue as the compiler would simply generate extra save/store of LR
> in the prologue/epilogue.
>
> OK for the trunk?
>
> Thanks,
> Yufeng
>
> gcc/testsuite/
>
> * gcc.target/aarch64/aapcs64/aapcs64.exp:
> (additional_flags_for_func_ret): New variable based on
> $additional_flags
> plus -fno-use-caller-save.
> (func-ret-*.c): Use the new variable.
OK.
Jeff
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH, AArch64, Testsuite] Specify -fno-use-caller-save for func-ret* tests
2014-07-01 17:26 ` Jeff Law
@ 2014-07-08 19:45 ` Tom de Vries
2014-07-11 13:22 ` Yufeng Zhang
0 siblings, 1 reply; 4+ messages in thread
From: Tom de Vries @ 2014-07-08 19:45 UTC (permalink / raw)
To: Jeff Law; +Cc: Yufeng Zhang, gcc-patches, Marcus Shawcroft, Richard Earnshaw
[-- Attachment #1: Type: text/plain, Size: 1361 bytes --]
On 01-07-14 19:26, Jeff Law wrote:
> On 07/01/14 09:51, Yufeng Zhang wrote:
>> Hi,
>>
>> This patch resolves a conflict between the aapcs64 test framework for
>> func-ret tests and the optimization option -fuse-caller-save, which was
>> enabled by default at -O1 or above recently.
>>
Minor detail: it's enabled by default at -O2 or above:
...
{ OPT_LEVELS_2_PLUS, OPT_fuse_caller_save, NULL, 1 },
...
>> Basically, the test framework has an inline-assembly based mechanism in
>> place which invokes the test facility function right on the return of a
>> tested function.
>> The compiler with -fuse-caller-save is unable to
>> identify the unconventional call graph and carries out the optimization
>> regardless.
AFAIU, we're overwriting the return register to implement a function call at
return in order to see the exact state of registers at return:
...
__attribute__ ((noinline)) unsigned char
func_return_val_0 (int i, double d, unsigned char t)
{
asm (""::"r" (i),"r" (d));
asm volatile ("mov %0, x30" : "=r" (saved_return_address));
asm volatile ("mov x30, %0" : : "r" ((unsigned long long) myfunc));
return t;
}
...
But we're not informing the compiler that a hidden function call takes place.
This patch fixes that, and there's no need to disable fuse-caller-save.
Tested with aarch64 build. OK for trunk?
Thanks,
- Tom
[-- Attachment #2: alternative-fix.patch --]
[-- Type: text/x-patch, Size: 2536 bytes --]
2014-07-08 Tom de Vries <tom@codesourcery.com>
* gcc.target/aarch64/aapcs64/aapcs64.exp
(additional_flags_for_func_ret): Remove.
(func-ret-*.c): Use additional_flags.
* gcc.target/aarch64/aapcs64/abitest-2.h (FUNC_VAL_CHECK): Add missing
call_used_regs clobbers.
Index: gcc/testsuite/gcc.target/aarch64/aapcs64/aapcs64.exp
===================================================================
--- gcc/testsuite/gcc.target/aarch64/aapcs64/aapcs64.exp (revision 212294)
+++ gcc/testsuite/gcc.target/aarch64/aapcs64/aapcs64.exp (working copy)
@@ -48,15 +48,11 @@ foreach src [lsort [glob -nocomplain $sr
}
# Test function return value.
-# Disable -fuse-caller-save to prevent the compiler from generating
-# conflicting code.
-set additional_flags_for_func_ret $additional_flags
-append additional_flags_for_func_ret " -fno-use-caller-save"
foreach src [lsort [glob -nocomplain $srcdir/$subdir/func-ret-*.c]] {
if {[runtest_file_p $runtests $src]} {
c-torture-execute [list $src \
$srcdir/$subdir/abitest.S] \
- $additional_flags_for_func_ret
+ $additional_flags
}
}
Index: gcc/testsuite/gcc.target/aarch64/aapcs64/abitest-2.h
===================================================================
--- gcc/testsuite/gcc.target/aarch64/aapcs64/abitest-2.h (revision 212294)
+++ gcc/testsuite/gcc.target/aarch64/aapcs64/abitest-2.h (working copy)
@@ -80,10 +80,18 @@ __attribute__ ((noinline)) type FUNC_NAM
The previous approach of sequentially calling myfunc right after \
this function does not guarantee myfunc see the exact register \
content, as compiler may emit code in between the two calls, \
- especially during the -O0 codegen. */ \
+ especially during the -O0 codegen. \
+ However, since we're doing a call, we need to clobber all call \
+ used regs. */ \
asm volatile ("mov %0, x30" : "=r" (saved_return_address)); \
- asm volatile ("mov x30, %0" : : "r" ((unsigned long long) myfunc)); \
- return t; \
+ asm volatile ("mov x30, %0" : : "r" ((unsigned long long) myfunc) : \
+ "x0", "x1", "x2", "x3", "x4", "x5", "x6", "x7", \
+ "x8", "x9", "x10", "x11", "x12", "x13", "x14", "x15", \
+ "x16", "x17", "x18", \
+ "v0", "v1", "v2", "v3", "v4", "v5", "v6", "v7", \
+ "v16", "v17", "v18", "v19", "v20", "v21", "v22", "v23", \
+ "v24", "v25", "v26", "v27", "v28", "v29", "v30", "v31");\
+ return t; \
}
#include TESTFILE
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH, AArch64, Testsuite] Specify -fno-use-caller-save for func-ret* tests
2014-07-08 19:45 ` Tom de Vries
@ 2014-07-11 13:22 ` Yufeng Zhang
0 siblings, 0 replies; 4+ messages in thread
From: Yufeng Zhang @ 2014-07-11 13:22 UTC (permalink / raw)
To: Tom de Vries; +Cc: Jeff Law, gcc-patches, Marcus Shawcroft, Richard Earnshaw
Hi Tom,
On 8 July 2014 20:45, Tom de Vries <Tom_deVries@mentor.com> wrote:
> On 01-07-14 19:26, Jeff Law wrote:
>>
>> On 07/01/14 09:51, Yufeng Zhang wrote:
>>>
>>> Hi,
>>>
>>> This patch resolves a conflict between the aapcs64 test framework for
>>> func-ret tests and the optimization option -fuse-caller-save, which was
>>> enabled by default at -O1 or above recently.
>>>
>
> Minor detail: it's enabled by default at -O2 or above:
> ...
> { OPT_LEVELS_2_PLUS, OPT_fuse_caller_save, NULL, 1 },
> ...
I see. Thanks for correcting me.
>
>
>>> Basically, the test framework has an inline-assembly based mechanism in
>>> place which invokes the test facility function right on the return of a
>>> tested function.
>
>>> The compiler with -fuse-caller-save is unable to
>>> identify the unconventional call graph and carries out the optimization
>>> regardless.
>
> AFAIU, we're overwriting the return register to implement a function call at
> return in order to see the exact state of registers at return:
Yes, exactly.
> ...
> __attribute__ ((noinline)) unsigned char
> func_return_val_0 (int i, double d, unsigned char t)
> {
> asm (""::"r" (i),"r" (d));
> asm volatile ("mov %0, x30" : "=r" (saved_return_address));
> asm volatile ("mov x30, %0" : : "r" ((unsigned long long) myfunc));
> return t;
> }
> ...
>
> But we're not informing the compiler that a hidden function call takes
> place. This patch fixes that, and there's no need to disable
> fuse-caller-save.
>
> Tested with aarch64 build. OK for trunk?
>
> Thanks,
> - Tom
>
> 2014-07-08 Tom de Vries <tom@codesourcery.com>
>
> * gcc.target/aarch64/aapcs64/aapcs64.exp
> (additional_flags_for_func_ret): Remove.
> (func-ret-*.c): Use additional_flags.
> * gcc.target/aarch64/aapcs64/abitest-2.h (FUNC_VAL_CHECK): Add missing
> call_used_regs clobbers.
>
> Index: gcc/testsuite/gcc.target/aarch64/aapcs64/aapcs64.exp
> ===================================================================
> --- gcc/testsuite/gcc.target/aarch64/aapcs64/aapcs64.exp (revision 212294)
> +++ gcc/testsuite/gcc.target/aarch64/aapcs64/aapcs64.exp (working copy)
> @@ -48,15 +48,11 @@ foreach src [lsort [glob -nocomplain $sr
> }
>
> # Test function return value.
> -# Disable -fuse-caller-save to prevent the compiler from generating
> -# conflicting code.
> -set additional_flags_for_func_ret $additional_flags
> -append additional_flags_for_func_ret " -fno-use-caller-save"
> foreach src [lsort [glob -nocomplain $srcdir/$subdir/func-ret-*.c]] {
> if {[runtest_file_p $runtests $src]} {
> c-torture-execute [list $src \
> $srcdir/$subdir/abitest.S] \
> - $additional_flags_for_func_ret
> + $additional_flags
> }
> }
>
> Index: gcc/testsuite/gcc.target/aarch64/aapcs64/abitest-2.h
> ===================================================================
> --- gcc/testsuite/gcc.target/aarch64/aapcs64/abitest-2.h (revision 212294)
> +++ gcc/testsuite/gcc.target/aarch64/aapcs64/abitest-2.h (working copy)
> @@ -80,10 +80,18 @@ __attribute__ ((noinline)) type FUNC_NAM
> The previous approach of sequentially calling myfunc right after \
> this function does not guarantee myfunc see the exact register \
> content, as compiler may emit code in between the two calls, \
> - especially during the -O0 codegen. */ \
> + especially during the -O0 codegen. \
> + However, since we're doing a call, we need to clobber all call \
> + used regs. */ \
> asm volatile ("mov %0, x30" : "=r" (saved_return_address)); \
> - asm volatile ("mov x30, %0" : : "r" ((unsigned long long) myfunc)); \
> - return t; \
> + asm volatile ("mov x30, %0" : : "r" ((unsigned long long) myfunc) : \
> + "x0", "x1", "x2", "x3", "x4", "x5", "x6", "x7", \
> + "x8", "x9", "x10", "x11", "x12", "x13", "x14", "x15", \
> + "x16", "x17", "x18", \
> + "v0", "v1", "v2", "v3", "v4", "v5", "v6", "v7", \
> + "v16", "v17", "v18", "v19", "v20", "v21", "v22", "v23", \
> + "v24", "v25", "v26", "v27", "v28", "v29", "v30", "v31");\
> + return t; \
> }
> #include TESTFILE
Your patch is probably OK (although I'm no longer in a position to
verify the code-gen by myself easily). I still prefer to have
-fuse-caller-save disabled for these tests in order to have a
strictly-conformed ABI environment for these ABI tests.
I'll leave the AArch64 maintainers to comment.
Thanks,
Yufeng
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-07-11 13:22 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-01 15:51 [PATCH, AArch64, Testsuite] Specify -fno-use-caller-save for func-ret* tests Yufeng Zhang
2014-07-01 17:26 ` Jeff Law
2014-07-08 19:45 ` Tom de Vries
2014-07-11 13:22 ` Yufeng Zhang
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).