This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]