Message ID | 53BC4A53.1020300@mentor.com |
---|---|
State | New |
Headers | show |
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
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