* [PATCH][PR target/81535] Fix tests on Power
@ 2017-07-28 4:42 Yury Gribov
2017-08-04 20:35 ` [PATCH][PING][PR " Yury Gribov
2017-08-07 23:33 ` [PATCH][PR " Segher Boessenkool
0 siblings, 2 replies; 4+ messages in thread
From: Yury Gribov @ 2017-07-28 4:42 UTC (permalink / raw)
To: gcc-patches; +Cc: seurer, wschmidt, meissner, marxin
[-- Attachment #1: Type: text/plain, Size: 513 bytes --]
Hi all,
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?
-Y
[-- Attachment #2: pr81535-1.patch --]
[-- Type: text/plain, Size: 2438 bytes --]
2017-07-28 Yury Gribov <tetra2005@gmail.com>
PR target/81535
* gcc.dg/pr56727-1.c: Do not check output on Power.
* gcc.dg/pr56727-2.c: Fix pattern for Power.
* 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); } \
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);
}
@@ -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* } } } */
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 } } */
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH][PING][PR target/81535] Fix tests on Power 2017-07-28 4:42 [PATCH][PR target/81535] Fix tests on Power Yury Gribov @ 2017-08-04 20:35 ` Yury Gribov 2017-08-07 23:33 ` [PATCH][PR " Segher Boessenkool 1 sibling, 0 replies; 4+ messages in thread From: Yury Gribov @ 2017-08-04 20:35 UTC (permalink / raw) To: GCC Patches; +Cc: seurer, wschmidt, meissner, marxin [-- Attachment #1: Type: text/plain, Size: 514 bytes --] Hi all, 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? -Y [-- Attachment #2: pr81535-1.patch --] [-- Type: text/plain, Size: 2439 bytes --] 2017-07-28 Yury Gribov <tetra2005@gmail.com> PR target/81535 * gcc.dg/pr56727-1.c: Do not check output on Power. * gcc.dg/pr56727-2.c: Fix pattern for Power. * 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); } \ 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); } @@ -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* } } } */ 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 } } */ ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][PR target/81535] Fix tests on Power 2017-07-28 4:42 [PATCH][PR target/81535] Fix tests on Power Yury Gribov 2017-08-04 20:35 ` [PATCH][PING][PR " Yury Gribov @ 2017-08-07 23:33 ` Segher Boessenkool 2017-11-25 10:00 ` Yury Gribov 1 sibling, 1 reply; 4+ messages in thread From: Segher Boessenkool @ 2017-08-07 23:33 UTC (permalink / raw) To: Yury Gribov; +Cc: gcc-patches, seurer, wschmidt, meissner, marxin 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][PR target/81535] Fix tests on Power 2017-08-07 23:33 ` [PATCH][PR " Segher Boessenkool @ 2017-11-25 10:00 ` Yury Gribov 0 siblings, 0 replies; 4+ messages in thread From: Yury Gribov @ 2017-11-25 10:00 UTC (permalink / raw) To: Segher Boessenkool; +Cc: GCC Patches, seurer, wschmidt, meissner, marxin On Tue, Aug 8, 2017 at 12:32 AM, Segher Boessenkool <segher@kernel.crashing.org> wrote: > Hi Yuri, > > Sorry I missed this. Please cc: me to prevent that from happening. Segher, Sorry, somehow I missed your reply! > 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. No, only tested standard powerpc64. I'll go check other targets then. >> 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*-*-* . Makes sense. >> * 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? Well, it was decided in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56727 that it would be a valid optimization because the only way to expose the difference would be through dlsym hackery. Note that original PowerPC use-case (reported in https://sourceware.org/bugzilla/show_bug.cgi?id=21116) would benefit from this optimization as because PLT call + indirection would be replaced by normal PC-relative call. >> 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. Well, this was a minor hack to prevent tailcalling, I thought it does not deserve a comment... >> @@ -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. Yes, setjmp will block tailcall optimization. Will fix the pattern, thanks! >> 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? TBH at this point I don't remember. I think call to g() didn't add anything to the test so I removed it. Let me comment on this once I retest the patch as discussed above. -Y ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-11-25 8:51 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-07-28 4:42 [PATCH][PR target/81535] Fix tests on Power Yury Gribov 2017-08-04 20:35 ` [PATCH][PING][PR " Yury Gribov 2017-08-07 23:33 ` [PATCH][PR " Segher Boessenkool 2017-11-25 10:00 ` Yury Gribov
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).