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

2014-07-11 Thread Yufeng Zhang
Hi Tom,

On 8 July 2014 20:45, Tom de Vries tom_devr...@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  t...@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


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

2014-07-08 Thread Tom de Vries

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

2014-07-08  Tom de Vries  t...@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
 


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

2014-07-01 Thread Yufeng Zhang

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.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
 }
 }
 

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

2014-07-01 Thread Jeff Law

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