* [PATCH] Fix up 20181120-1.c testcase on big-endian (PR rtl-optimization/85925)
@ 2018-11-21 13:14 Jakub Jelinek
2018-11-21 17:24 ` Segher Boessenkool
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2018-11-21 13:14 UTC (permalink / raw)
To: Rainer Orth, Mike Stump; +Cc: gcc-patches
Hi!
As mentioned in the PR, the testcase fails on big-endian targets.
The following patch tweaks it so that it does not fail there and still
checks for the original bug.
Tested on x86_64-linux and i686-linux, ok for trunk and release branches?
2018-11-21 Jakub Jelinek <jakub@redhat.com>
PR rtl-optimization/85925
* gcc.c-torture/execute/20181120-1.c (u): New variable.
(main): Compare d against u.f1 rather than 0x101.
--- gcc/testsuite/gcc.c-torture/execute/20181120-1.c.jj 2018-11-20 21:39:05.230507352 +0100
+++ gcc/testsuite/gcc.c-torture/execute/20181120-1.c 2018-11-21 11:49:29.919488909 +0100
@@ -9,6 +9,7 @@ union U1 {
unsigned f0;
unsigned f1 : 15;
};
+volatile union U1 u = { 0x10101 };
int main (void)
{
@@ -19,7 +20,7 @@ int main (void)
*e = f.f1;
}
- if (d != 0x101)
+ if (d != u.f1)
__builtin_abort ();
return 0;
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix up 20181120-1.c testcase on big-endian (PR rtl-optimization/85925)
2018-11-21 13:14 [PATCH] Fix up 20181120-1.c testcase on big-endian (PR rtl-optimization/85925) Jakub Jelinek
@ 2018-11-21 17:24 ` Segher Boessenkool
2018-11-21 17:31 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Segher Boessenkool @ 2018-11-21 17:24 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Rainer Orth, Mike Stump, gcc-patches
Hi,
On Wed, Nov 21, 2018 at 02:13:55PM +0100, Jakub Jelinek wrote:
> As mentioned in the PR, the testcase fails on big-endian targets.
> The following patch tweaks it so that it does not fail there and still
> checks for the original bug.
It relies on a certain bitfield layout, not just on LE. I think the
testcase should run only on those specific targets where it works. I don't
see how this patch would fix the problem for BE, btw.
Segher
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix up 20181120-1.c testcase on big-endian (PR rtl-optimization/85925)
2018-11-21 17:24 ` Segher Boessenkool
@ 2018-11-21 17:31 ` Jakub Jelinek
2018-11-21 18:08 ` Segher Boessenkool
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2018-11-21 17:31 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Rainer Orth, Mike Stump, gcc-patches
On Wed, Nov 21, 2018 at 11:23:38AM -0600, Segher Boessenkool wrote:
> Hi,
>
> On Wed, Nov 21, 2018 at 02:13:55PM +0100, Jakub Jelinek wrote:
> > As mentioned in the PR, the testcase fails on big-endian targets.
> > The following patch tweaks it so that it does not fail there and still
> > checks for the original bug.
>
> It relies on a certain bitfield layout, not just on LE. I think the
> testcase should run only on those specific targets where it works. I don't
> see how this patch would fix the problem for BE, btw.
With the patch, it doesn't rely on anything, it compares if what you get at
runtime from the code combiner would optimize is equal to what is read from
a volatile union.
Admittedly, it might be better if the initializer was 0x1010101 or say
0x4030201 because on big endian in particular 0x10101 has the top 15 bits
all zero and thus that is what is in u.f1, so if the bug can be reproduced
with the combine.c + rtlanal.c fix reverted with 0x4030201, it would be
better to use that value (in both spots).
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix up 20181120-1.c testcase on big-endian (PR rtl-optimization/85925)
2018-11-21 17:31 ` Jakub Jelinek
@ 2018-11-21 18:08 ` Segher Boessenkool
2018-11-21 19:12 ` [PATCH] Fix up 20181120-1.c testcase on big-endian (PR rtl-optimization/85925, take 2) Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Segher Boessenkool @ 2018-11-21 18:08 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Rainer Orth, Mike Stump, gcc-patches
On Wed, Nov 21, 2018 at 06:31:43PM +0100, Jakub Jelinek wrote:
> > > As mentioned in the PR, the testcase fails on big-endian targets.
> > > The following patch tweaks it so that it does not fail there and still
> > > checks for the original bug.
> >
> > It relies on a certain bitfield layout, not just on LE. I think the
> > testcase should run only on those specific targets where it works. I don't
> > see how this patch would fix the problem for BE, btw.
>
> With the patch, it doesn't rely on anything, it compares if what you get at
> runtime from the code combiner would optimize is equal to what is read from
> a volatile union.
Oh, I think I misread it, sorry :-)
> Admittedly, it might be better if the initializer was 0x1010101 or say
> 0x4030201 because on big endian in particular 0x10101 has the top 15 bits
> all zero and thus that is what is in u.f1, so if the bug can be reproduced
> with the combine.c + rtlanal.c fix reverted with 0x4030201, it would be
> better to use that value (in both spots).
Yeah good point.
Segher
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] Fix up 20181120-1.c testcase on big-endian (PR rtl-optimization/85925, take 2)
2018-11-21 18:08 ` Segher Boessenkool
@ 2018-11-21 19:12 ` Jakub Jelinek
2018-11-21 20:38 ` Segher Boessenkool
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2018-11-21 19:12 UTC (permalink / raw)
To: Segher Boessenkool, Eric Botcazou, Rainer Orth, Mike Stump; +Cc: gcc-patches
Hi!
On Wed, Nov 21, 2018 at 12:07:51PM -0600, Segher Boessenkool wrote:
> > Admittedly, it might be better if the initializer was 0x1010101 or say
> > 0x4030201 because on big endian in particular 0x10101 has the top 15 bits
> > all zero and thus that is what is in u.f1, so if the bug can be reproduced
> > with the combine.c + rtlanal.c fix reverted with 0x4030201, it would be
> > better to use that value (in both spots).
>
> Yeah good point.
I've now managed to test this with a cross to armv7hl (scped to an arm box)
with and without the rtlanal.c + combine.c change reverted and on
powerpc64-linux as example of big-endian, on armv7hl it still fails with
the changes reverted, otherwise it succeeds on both. The test also needs
32-bit int target (previously just 17-bit or more, so I've added effective
target).
Ok for trunk and release branches?
2018-11-21 Jakub Jelinek <jakub@redhat.com>
PR rtl-optimization/85925
* gcc.c-torture/execute/20181120-1.c: Require effective target
int32plus.
(u): New variable.
(main): Compare d against u.f1 rather than 0x101. Use 0x4030201
instead of 0x10101.
--- gcc/testsuite/gcc.c-torture/execute/20181120-1.c.jj 2018-11-21 17:39:47.963671708 +0100
+++ gcc/testsuite/gcc.c-torture/execute/20181120-1.c 2018-11-21 20:07:45.804556443 +0100
@@ -1,4 +1,5 @@
/* PR rtl-optimization/85925 */
+/* { dg-require-effective-target int32plus } */
/* Testcase by <sudi@gcc.gnu.org> */
int a, c, d;
@@ -9,17 +10,18 @@ union U1 {
unsigned f0;
unsigned f1 : 15;
};
+volatile union U1 u = { 0x4030201 };
int main (void)
{
for (c = 0; c <= 1; c++) {
- union U1 f = {0x10101};
+ union U1 f = {0x4030201};
if (c == 1)
b;
*e = f.f1;
}
- if (d != 0x101)
+ if (d != u.f1)
__builtin_abort ();
return 0;
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix up 20181120-1.c testcase on big-endian (PR rtl-optimization/85925, take 2)
2018-11-21 19:12 ` [PATCH] Fix up 20181120-1.c testcase on big-endian (PR rtl-optimization/85925, take 2) Jakub Jelinek
@ 2018-11-21 20:38 ` Segher Boessenkool
0 siblings, 0 replies; 6+ messages in thread
From: Segher Boessenkool @ 2018-11-21 20:38 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Eric Botcazou, Rainer Orth, Mike Stump, gcc-patches
On Wed, Nov 21, 2018 at 08:12:44PM +0100, Jakub Jelinek wrote:
> On Wed, Nov 21, 2018 at 12:07:51PM -0600, Segher Boessenkool wrote:
> > > Admittedly, it might be better if the initializer was 0x1010101 or say
> > > 0x4030201 because on big endian in particular 0x10101 has the top 15 bits
> > > all zero and thus that is what is in u.f1, so if the bug can be reproduced
> > > with the combine.c + rtlanal.c fix reverted with 0x4030201, it would be
> > > better to use that value (in both spots).
> >
> > Yeah good point.
>
> I've now managed to test this with a cross to armv7hl (scped to an arm box)
> with and without the rtlanal.c + combine.c change reverted and on
> powerpc64-linux as example of big-endian, on armv7hl it still fails with
> the changes reverted, otherwise it succeeds on both. The test also needs
> 32-bit int target (previously just 17-bit or more, so I've added effective
> target).
It fixes the problem on powerpc64-linux {-m32,-m64}. Thanks :-)
Segher
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-11-21 20:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-21 13:14 [PATCH] Fix up 20181120-1.c testcase on big-endian (PR rtl-optimization/85925) Jakub Jelinek
2018-11-21 17:24 ` Segher Boessenkool
2018-11-21 17:31 ` Jakub Jelinek
2018-11-21 18:08 ` Segher Boessenkool
2018-11-21 19:12 ` [PATCH] Fix up 20181120-1.c testcase on big-endian (PR rtl-optimization/85925, take 2) Jakub Jelinek
2018-11-21 20:38 ` Segher Boessenkool
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).