On Thu, Mar 11, 2021 at 9:19 AM Alexandre Oliva <ol...@adacore.com> wrote:
>
>
> Several ipa-sra tests fail because -mlongcall on powerpc is a
> TYPE_ATTRIBUTE, and those disable ipa-sra.
>
> This local workaround disables -mlongcall for the failing tests on
> powerpc*-*-vxworks*, so they get a chance to run even in kernel mode.
>
> (AdaCore has -mlongcall enabled for powerpc*-wrs-vxworks* kernel-mode
> targets.)
>
> This was regstrapped on x86_64-linux-gnu and ppc64-linux-gnu, and tested
> with a cross to a ppc64-vxworks7r2 with -mlongcall enabled by default.
>
> I was leaning towards maintaing this change internal, but I thought it
> wouldn't hurt to ask whether it's ok to install.
>
> I wonder whether it wouldn't be more desirable to have an alternate
> implementation of -mlongcall on ppc that didn't conflict with IPA-SRA,
> and IIRC with some scenarios involving C++ templates.
>
> E.g., on ARM, -mlong-calls doesn't add an attribute to every function,
> it just changes a default, that attributes short_call and long_call may
> then override for specific functions. Would a similar implementation be
> acceptable for ppc?
I wonder what the effect of this switch is - it's documented as
affecting the call site (different call insn sequence by default), so
the best representation would be an attribute on actual call stmts so
inlining a non -mlongcall fn into a -mlongcall fn will properly
inherit the setting of calls in the inlined function?
So in C++ I should be able to write
[[gnu::arm::longcall]] foo ();
to get a single call forced as long-call (whatever this is used for
in practice).
I agree that a type attribute (it's a type attribute likely to also
affect indirect calls) is a bit awkward but it does look better
than just a global flag.
Note that the long-standing issue in IPA is that IPA would need
to know the semantics of attributes since it might need to modify
them. One idea (partly implemented already I think) is to have
a whitelist, aka, "this attribute doesn't need to be adjusted when
parameters are split, removed or added or when the function is
split or merged" - where for target specific attributes this would
need a new target hook.
Adjusting the testsuite just hides this missed optimization issue,
so at least a bugreport with xfail keyword refering to the altered
tests should be opened.
Richard.
>
>
> for gcc/testsuite/ChangeLog
>
> * gcc.dg/ipa/ipa-sra-12.c: Add -mno-longcall on ppc*-vxworks*.
> * gcc.dg/ipa/ipa-sra-13.c: Likewise.
> * gcc.dg/ipa/ipa-sra-15.c: Likewise.
> * gcc.dg/ipa/ipa-sra-16.c: Likewise.
> * gcc.dg/ipa/ipa-sra-17.c: Likewise.
> * gcc.dg/ipa/ipa-sra-18.c: Likewise.
> ---
> gcc/testsuite/gcc.dg/ipa/ipa-sra-12.c | 1 +
> gcc/testsuite/gcc.dg/ipa/ipa-sra-13.c | 1 +
> gcc/testsuite/gcc.dg/ipa/ipa-sra-15.c | 1 +
> gcc/testsuite/gcc.dg/ipa/ipa-sra-16.c | 1 +
> gcc/testsuite/gcc.dg/ipa/ipa-sra-17.c | 1 +
> gcc/testsuite/gcc.dg/ipa/ipa-sra-18.c | 1 +
> 6 files changed, 6 insertions(+)
>
> diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-sra-12.c
> b/gcc/testsuite/gcc.dg/ipa/ipa-sra-12.c
> index 0cc76bde319c7..0d74c8f52eee2 100644
> --- a/gcc/testsuite/gcc.dg/ipa/ipa-sra-12.c
> +++ b/gcc/testsuite/gcc.dg/ipa/ipa-sra-12.c
> @@ -1,5 +1,6 @@
> /* { dg-do run } */
> /* { dg-options "-O2 -fipa-sra -fdump-ipa-sra" } */
> +/* { dg-additional-options "-mno-longcall" { target powerpc*-*-vxworks* } }
> */
>
> /* Check of a simple and transitive structure split. */
>
> diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-sra-13.c
> b/gcc/testsuite/gcc.dg/ipa/ipa-sra-13.c
> index e8751dad67fcc..66083f040bad9 100644
> --- a/gcc/testsuite/gcc.dg/ipa/ipa-sra-13.c
> +++ b/gcc/testsuite/gcc.dg/ipa/ipa-sra-13.c
> @@ -1,5 +1,6 @@
> /* { dg-do run } */
> /* { dg-options "-O2 -fipa-sra -fdump-ipa-sra" } */
> +/* { dg-additional-options "-mno-longcall" { target powerpc*-*-vxworks* } }
> */
>
> /* Check of a by-reference structure split. */
>
> diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-sra-15.c
> b/gcc/testsuite/gcc.dg/ipa/ipa-sra-15.c
> index aa13a94c7c064..98b7ae5d4f11b 100644
> --- a/gcc/testsuite/gcc.dg/ipa/ipa-sra-15.c
> +++ b/gcc/testsuite/gcc.dg/ipa/ipa-sra-15.c
> @@ -1,5 +1,6 @@
> /* { dg-do run } */
> /* { dg-options "-O2 -fipa-sra -fdump-ipa-sra" } */
> +/* { dg-additional-options "-mno-longcall" { target powerpc*-*-vxworks* } }
> */
>
> /* Check of a recursive by-reference structure split. The recursive
> functions
> have to be pure right from the start, otherwise the current AA would
> detect
> diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-sra-16.c
> b/gcc/testsuite/gcc.dg/ipa/ipa-sra-16.c
> index 2bffe297c7456..25f65fe1fc05a 100644
> --- a/gcc/testsuite/gcc.dg/ipa/ipa-sra-16.c
> +++ b/gcc/testsuite/gcc.dg/ipa/ipa-sra-16.c
> @@ -1,5 +1,6 @@
> /* { dg-do compile } */
> /* { dg-options "-O2 -fipa-sra -fdump-ipa-sra -fdump-tree-optimized" } */
> +/* { dg-additional-options "-mno-longcall" { target powerpc*-*-vxworks* } }
> */
>
> /* Testing removal of unused parameters in recursive calls. */
>
> diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-sra-17.c
> b/gcc/testsuite/gcc.dg/ipa/ipa-sra-17.c
> index 9cb6367b37478..eaec0fcf90a91 100644
> --- a/gcc/testsuite/gcc.dg/ipa/ipa-sra-17.c
> +++ b/gcc/testsuite/gcc.dg/ipa/ipa-sra-17.c
> @@ -1,5 +1,6 @@
> /* { dg-do compile } */
> /* { dg-options "-O2 -fdump-ipa-sra -fdump-tree-optimized" } */
> +/* { dg-additional-options "-mno-longcall" { target powerpc*-*-vxworks* } }
> */
>
> #define DOIT
> #define DONT
> diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-sra-18.c
> b/gcc/testsuite/gcc.dg/ipa/ipa-sra-18.c
> index 3217b612231a2..1435ab228e3ae 100644
> --- a/gcc/testsuite/gcc.dg/ipa/ipa-sra-18.c
> +++ b/gcc/testsuite/gcc.dg/ipa/ipa-sra-18.c
> @@ -1,5 +1,6 @@
> /* { dg-do compile } */
> /* { dg-options "-O2 -fdump-ipa-sra" } */
> +/* { dg-additional-options "-mno-longcall" { target powerpc*-*-vxworks* } }
> */
>
> struct S
> {
>
> --
> Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/
> Free Software Activist GNU Toolchain Engineer
> Vim, Vi, Voltei pro Emacs -- GNUlius Caesar
