From: Segher Boessenkool <segher@kernel.crashing.org>
To: Yury Gribov <tetra2005.patches@gmail.com>
Cc: gcc-patches@gcc.gnu.org, seurer@gcc.gnu.org,
wschmidt@gcc.gnu.org, meissner@gcc.gnu.org,
marxin <marxin@gcc.gnu.org>
Subject: Re: [PATCH][PR target/81535] Fix tests on Power
Date: Mon, 07 Aug 2017 23:33:00 -0000 [thread overview]
Message-ID: <20170807233257.GK13471@gate.crashing.org> (raw)
In-Reply-To: <2bbe67af-554a-30f4-f391-6282ab65c7ef@gmail.com>
Hi Yuri,
Sorry I missed this. Please cc: me to prevent that from happening.
On Fri, Jul 28, 2017 at 05:42:00AM +0100, Yury Gribov wrote:
> This patch fixes issues reported in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81535
>
> I removed call to g in pr79439.c because gcc was duplicating the basic
> block with call depending on compiler flags (so scan-assembler-times
> pattern wasn't reliable anymore). I also added alias to prevent
> inlining introduced by recent PR56727 patch.
>
> I added Power-specific pattern in pr56727-2.c testcase and disabled
> testing on Power in pr56727-1.c.
>
> Tested on powerpc64-unknown-linux-gnu. Ok for trunk?
Did you also test this with -m32? And on powerpc64le-linux, the target
the bug is reported against? The three ABIs are different.
> PR target/81535
> * gcc.dg/pr56727-1.c: Do not check output on Power.
> * gcc.dg/pr56727-2.c: Fix pattern for Power.
Please name the actual target triple here, i.e. powerpc*-*-* .
> * gcc.target/powerpc/pr79439.c: Prevent inlining.
> diff -rupN gcc/gcc/testsuite/gcc.dg/pr56727-1.c gcc-81535/gcc/testsuite/gcc.dg/pr56727-1.c
> --- gcc/gcc/testsuite/gcc.dg/pr56727-1.c 2017-07-28 02:39:54.770046466 +0000
> +++ gcc-81535/gcc/testsuite/gcc.dg/pr56727-1.c 2017-07-28 04:25:04.805648587 +0000
> @@ -1,6 +1,6 @@
> /* { dg-do compile { target fpic } } */
> /* { dg-options "-O2 -fPIC" } */
> -/* { dg-final { scan-assembler-not "@(PLT|plt)" { target i?86-*-* x86_64-*-* powerpc*-*-* } } } */
> +/* { dg-final { scan-assembler-not "@(PLT|plt)" { target i?86-*-* x86_64-*-* } } } */
>
> #define define_func(type) \
> void f_ ## type (type b) { f_ ## type (0); } \
Is it correct that current GCC does not do the call via the PLT?
> diff -rupN gcc/gcc/testsuite/gcc.dg/pr56727-2.c gcc-81535/gcc/testsuite/gcc.dg/pr56727-2.c
> --- gcc/gcc/testsuite/gcc.dg/pr56727-2.c 2017-07-28 02:39:54.770046466 +0000
> +++ gcc-81535/gcc/testsuite/gcc.dg/pr56727-2.c 2017-07-28 04:21:19.195215187 +0000
> @@ -1,10 +1,10 @@
> /* { dg-do compile { target fpic } } */
> /* { dg-options "-O2 -fPIC" } */
> -/* { dg-final { scan-assembler "@(PLT|plt)" { target i?86-*-* x86_64-*-* powerpc*-*-linux* } } } */
>
> __attribute__((noinline, noclone))
> void f (short b)
> {
> + __builtin_setjmp (0); /* Prevent tailcall */
> f (0);
> }
>
This change is not mentioned in the changelog.
> @@ -14,3 +14,5 @@ void h ()
> {
> g (0);
> }
> +/* { dg-final { scan-assembler "@(PLT|plt)" { target i?86-*-* x86_64-*-* } } } */
> +/* { dg-final { scan-assembler "bl f\n\[ \t\]*nop" { target powerpc*-*-linux* } } } */
Is there a real reason there cannot be a tailcall here? You can write
this as { scan-assembler {\mbl f\s+nop\M} } btw.
> diff -rupN gcc/gcc/testsuite/gcc.target/powerpc/pr79439.c gcc-81535/gcc/testsuite/gcc.target/powerpc/pr79439.c
> --- gcc/gcc/testsuite/gcc.target/powerpc/pr79439.c 2017-07-28 02:39:55.750048426 +0000
> +++ gcc-81535/gcc/testsuite/gcc.target/powerpc/pr79439.c 2017-07-28 04:13:47.834177237 +0000
> @@ -8,22 +8,17 @@
>
> int f (void);
>
> -void
> -g (void)
> -{
> -}
> -
> int
> rec (int a)
> {
> int ret = 0;
> if (a > 10 && f ())
> ret += rec (a - 1);
> - g ();
> return a + ret;
> }
>
> +void rec_alias (short) __attribute__ ((alias ("rec")));
> +
> /* { dg-final { scan-assembler-times {\mbl f\M} 1 } } */
> -/* { dg-final { scan-assembler-times {\mbl g\M} 1 } } */
> /* { dg-final { scan-assembler-times {\mbl rec\M} 1 } } */
> -/* { dg-final { scan-assembler-times {\mnop\M} 3 } } */
> +/* { dg-final { scan-assembler-times {\mnop\M} 2 } } */
The changelog says "prevent inlining", but you actually delete the code.
Is that okay, wasn't the testcase actually testing for something here?
Segher
next prev parent reply other threads:[~2017-08-07 23:33 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-28 4:42 Yury Gribov
2017-08-04 20:35 ` [PATCH][PING][PR " Yury Gribov
2017-08-07 23:33 ` Segher Boessenkool [this message]
2017-11-25 10:00 ` [PATCH][PR " Yury Gribov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170807233257.GK13471@gate.crashing.org \
--to=segher@kernel.crashing.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=marxin@gcc.gnu.org \
--cc=meissner@gcc.gnu.org \
--cc=seurer@gcc.gnu.org \
--cc=tetra2005.patches@gmail.com \
--cc=wschmidt@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).