diff mbox

[AArch64,Testsuite] Specify -fno-use-caller-save for func-ret* tests

Message ID 53BC4A53.1020300@mentor.com
State New
Headers show

Commit Message

Tom de Vries July 8, 2014, 7:45 p.m. UTC
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

Comments

Yufeng Zhang July 11, 2014, 1:22 p.m. UTC | #1
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
diff mbox

Patch

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