Re: [PATCH 1/2] simplify-rtx: The truncation of an IOR can have all bits set (PR81423)
On 08/07/2017 04:33 PM, Segher Boessenkool wrote: > On Tue, Jul 25, 2017 at 06:25:49AM -0500, Segher Boessenkool wrote: >> On Mon, Jul 24, 2017 at 04:06:39PM -0600, Jeff Law wrote: 2017-07-24 Segher Boessenkool gcc/testsuite/ PR rtl-optimization/81423 * gcc.c-torture/execute/pr81423.c: New testcase. >>> I think int32plus just indicates ints are at least 32 bits. But a long >>> or long long could still be just 32 bits. so int32plus && long_neq_int, >>> to ensure that long/long long are 64 bits? >> >> Well, long long is required to be 64 bits or more by the C standard. >> But some GCC targets do not follow that, with certain options at least. >> >> It looks like that test actually requires long long to be *exactly* >> 64 bits. I'll modify the test to test for that. > > So I came up with the following. Is this okay for trunk? (Tested on > powerpc64-linux and x86_64-linux, both with both -m32 and -m64, and > tested it does fail on x86 without the patches to fix the bug). > > > Segher > > > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr81423.c > b/gcc/testsuite/gcc.c-torture/execute/pr81423.c > new file mode 100644 > index 000..731aa8f > --- /dev/null > +++ b/gcc/testsuite/gcc.c-torture/execute/pr81423.c > @@ -0,0 +1,36 @@ > +extern void abort (void); > + > +unsigned long long int ll = 0; > +unsigned long long int ull1 = 1ULL; > +unsigned long long int ull2 = 12008284144813806346ULL; > +unsigned long long int ull3; > + > +unsigned long long int __attribute__ ((noinline)) > +foo (void) > +{ > + ll = -5597998501375493990LL; > + > + ll = (5677365550390624949L - ll) - (ull1 > 0); > + unsigned long long int ull3; > + ull3 = (unsigned int) > +(2067854353L << > + (((ll + -2129105131L) ^ 10280750144413668236ULL) - > + 10280750143997242009ULL)) >> ((2873442921854271231ULL | ull2) > + - 12098357307243495419ULL); > + > + return ull3; > +} > + > +int > +main (void) > +{ > + /* We need a long long of exactly 64 bits for this test. */ > + ll--; > + if (ll != 0xULL) > +return 0; I think we've used sizeof to check this in the past. But I'm not wed to that approach. If it's working for you, then let's go with it. jeff
Re: [PATCH 1/2] simplify-rtx: The truncation of an IOR can have all bits set (PR81423)
On Tue, Jul 25, 2017 at 06:25:49AM -0500, Segher Boessenkool wrote: > On Mon, Jul 24, 2017 at 04:06:39PM -0600, Jeff Law wrote: > > > 2017-07-24 Segher Boessenkool > > > > > > gcc/testsuite/ > > > PR rtl-optimization/81423 > > > * gcc.c-torture/execute/pr81423.c: New testcase. > > I think int32plus just indicates ints are at least 32 bits. But a long > > or long long could still be just 32 bits. so int32plus && long_neq_int, > > to ensure that long/long long are 64 bits? > > Well, long long is required to be 64 bits or more by the C standard. > But some GCC targets do not follow that, with certain options at least. > > It looks like that test actually requires long long to be *exactly* > 64 bits. I'll modify the test to test for that. So I came up with the following. Is this okay for trunk? (Tested on powerpc64-linux and x86_64-linux, both with both -m32 and -m64, and tested it does fail on x86 without the patches to fix the bug). Segher diff --git a/gcc/testsuite/gcc.c-torture/execute/pr81423.c b/gcc/testsuite/gcc.c-torture/execute/pr81423.c new file mode 100644 index 000..731aa8f --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr81423.c @@ -0,0 +1,36 @@ +extern void abort (void); + +unsigned long long int ll = 0; +unsigned long long int ull1 = 1ULL; +unsigned long long int ull2 = 12008284144813806346ULL; +unsigned long long int ull3; + +unsigned long long int __attribute__ ((noinline)) +foo (void) +{ + ll = -5597998501375493990LL; + + ll = (5677365550390624949L - ll) - (ull1 > 0); + unsigned long long int ull3; + ull3 = (unsigned int) +(2067854353L << + (((ll + -2129105131L) ^ 10280750144413668236ULL) - + 10280750143997242009ULL)) >> ((2873442921854271231ULL | ull2) + - 12098357307243495419ULL); + + return ull3; +} + +int +main (void) +{ + /* We need a long long of exactly 64 bits for this test. */ + ll--; + if (ll != 0xULL) +return 0; + + ull3 = foo (); + if (ull3 != 3998784) +abort (); + return 0; +}
Re: [PATCH 1/2] simplify-rtx: The truncation of an IOR can have all bits set (PR81423)
On Jul 25, 2017, at 3:10 PM, Segher Boessenkool wrote: > > On Tue, Jul 25, 2017 at 12:30:13PM +0100, Kyrill Tkachov wrote: >> We sometimes use the __mode__ attribute to force certain sizes in C types. >> For example: typedef int ditype __attribute__ ((mode (DI))); >> Maybe you can do this to force the right sizes. I don't know if there are >> any targets >> that don't support DImode ops though :) > > DImode isn't necessarily the same size on all targets, a byte isn't > always eight bits. As a practical matter, presently a byte is always eight bits and a DI is always 8 bytes in gcc. :-) Pretending otherwise is a fool's errand. We like to kid ourselves that a character isn't always 8 bits, but the first person to want to do that will discover the lie it is. smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH 1/2] simplify-rtx: The truncation of an IOR can have all bits set (PR81423)
On Tue, Jul 25, 2017 at 12:30:13PM +0100, Kyrill Tkachov wrote: > We sometimes use the __mode__ attribute to force certain sizes in C types. > For example: typedef int ditype __attribute__ ((mode (DI))); > Maybe you can do this to force the right sizes. I don't know if there are > any targets > that don't support DImode ops though :) DImode isn't necessarily the same size on all targets, a byte isn't always eight bits. I was planning on just doing some ugly #if. Segher
Re: [PATCH 1/2] simplify-rtx: The truncation of an IOR can have all bits set (PR81423)
On 24/07/17 09:50, Segher Boessenkool wrote: On Wed, Jul 19, 2017 at 12:19:32AM -0600, Jeff Law wrote: On 07/18/2017 01:36 PM, Segher Boessenkool wrote: * simplify-rtx.c (simplify_truncation): Handle truncating an IOR with a constant that is -1 in the truncated to mode. OK. A testcase would be advisable :-) jeff Like this. Is this okay for trunk? (Is int32plus the correct test to use here?) We sometimes use the __mode__ attribute to force certain sizes in C types. For example: typedef int ditype __attribute__ ((mode (DI))); Maybe you can do this to force the right sizes. I don't know if there are any targets that don't support DImode ops though :) Kyrill Segher 2017-07-24 Segher Boessenkool gcc/testsuite/ PR rtl-optimization/81423 * gcc.c-torture/execute/pr81423.c: New testcase. --- gcc/testsuite/gcc.c-torture/execute/pr81423.c | 30 +++ 1 file changed, 30 insertions(+) create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr81423.c diff --git a/gcc/testsuite/gcc.c-torture/execute/pr81423.c b/gcc/testsuite/gcc.c-torture/execute/pr81423.c new file mode 100644 index 000..75c3518 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr81423.c @@ -0,0 +1,30 @@ +/* { dg-require-effective-target int32plus } */ + +extern void abort (void); + +unsigned long long int ll = 0; +unsigned long long int ull1 = 1ULL; +unsigned long long int ull2 = 12008284144813806346ULL; +unsigned long long int ull3; + +void +foo (void) +{ + ll = -5597998501375493990LL; + + ll = (5677365550390624949L - ll) - (ull1 > 0); + ull3 = (unsigned int) +(2067854353L << + (((ll + -2129105131L) ^ 10280750144413668236ULL) - + 10280750143997242009ULL)) >> ((2873442921854271231ULL | ull2) + - 12098357307243495419ULL); +} + +int +main (void) +{ + foo (); + if (ull3 != 3998784) +abort (); + return 0; +}
Re: [PATCH 1/2] simplify-rtx: The truncation of an IOR can have all bits set (PR81423)
On Mon, Jul 24, 2017 at 04:06:39PM -0600, Jeff Law wrote: > > 2017-07-24 Segher Boessenkool > > > > gcc/testsuite/ > > PR rtl-optimization/81423 > > * gcc.c-torture/execute/pr81423.c: New testcase. > I think int32plus just indicates ints are at least 32 bits. But a long > or long long could still be just 32 bits. so int32plus && long_neq_int, > to ensure that long/long long are 64 bits? Well, long long is required to be 64 bits or more by the C standard. But some GCC targets do not follow that, with certain options at least. It looks like that test actually requires long long to be *exactly* 64 bits. I'll modify the test to test for that. Segher
Re: [PATCH 1/2] simplify-rtx: The truncation of an IOR can have all bits set (PR81423)
On 07/24/2017 02:50 AM, Segher Boessenkool wrote: > On Wed, Jul 19, 2017 at 12:19:32AM -0600, Jeff Law wrote: >> On 07/18/2017 01:36 PM, Segher Boessenkool wrote: >>> * simplify-rtx.c (simplify_truncation): Handle truncating an IOR >>> with a constant that is -1 in the truncated to mode. >> OK. A testcase would be advisable :-) >> >> jeff > > Like this. Is this okay for trunk? (Is int32plus the correct test > to use here?) > > > Segher > > > 2017-07-24 Segher Boessenkool > > gcc/testsuite/ > PR rtl-optimization/81423 > * gcc.c-torture/execute/pr81423.c: New testcase. I think int32plus just indicates ints are at least 32 bits. But a long or long long could still be just 32 bits. so int32plus && long_neq_int, to ensure that long/long long are 64 bits? OK with that change. jeff
Re: [PATCH 1/2] simplify-rtx: The truncation of an IOR can have all bits set (PR81423)
On Wed, Jul 19, 2017 at 12:19:32AM -0600, Jeff Law wrote: > On 07/18/2017 01:36 PM, Segher Boessenkool wrote: > > * simplify-rtx.c (simplify_truncation): Handle truncating an IOR > > with a constant that is -1 in the truncated to mode. > OK. A testcase would be advisable :-) > > jeff Like this. Is this okay for trunk? (Is int32plus the correct test to use here?) Segher 2017-07-24 Segher Boessenkool gcc/testsuite/ PR rtl-optimization/81423 * gcc.c-torture/execute/pr81423.c: New testcase. --- gcc/testsuite/gcc.c-torture/execute/pr81423.c | 30 +++ 1 file changed, 30 insertions(+) create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr81423.c diff --git a/gcc/testsuite/gcc.c-torture/execute/pr81423.c b/gcc/testsuite/gcc.c-torture/execute/pr81423.c new file mode 100644 index 000..75c3518 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr81423.c @@ -0,0 +1,30 @@ +/* { dg-require-effective-target int32plus } */ + +extern void abort (void); + +unsigned long long int ll = 0; +unsigned long long int ull1 = 1ULL; +unsigned long long int ull2 = 12008284144813806346ULL; +unsigned long long int ull3; + +void +foo (void) +{ + ll = -5597998501375493990LL; + + ll = (5677365550390624949L - ll) - (ull1 > 0); + ull3 = (unsigned int) +(2067854353L << + (((ll + -2129105131L) ^ 10280750144413668236ULL) - + 10280750143997242009ULL)) >> ((2873442921854271231ULL | ull2) + - 12098357307243495419ULL); +} + +int +main (void) +{ + foo (); + if (ull3 != 3998784) +abort (); + return 0; +}
Re: [PATCH 1/2] simplify-rtx: The truncation of an IOR can have all bits set (PR81423)
On Wed, Jul 19, 2017 at 12:19:32AM -0600, Jeff Law wrote: > On 07/18/2017 01:36 PM, Segher Boessenkool wrote: > > ... if it is an IOR with a constant with all bits set in the mode > > that is truncated to, for example. Handle that case. > > > > With this patch the problematic situation for the PR81423 testcase > > isn't even reached; but the next patch fixes that anyway. > > > > Bootstrapped and tested on powerpc64-linux {-m32,-m64} and on > > x86_64-linux. Is this okay for trunk? > > > > > > Segher > > > > > > 2017-07-18 Segher Boessenkool > > > > PR rtl-optimization/81423 > > * simplify-rtx.c (simplify_truncation): Handle truncating an IOR > > with a constant that is -1 in the truncated to mode. > OK. A testcase would be advisable :-) Thanks. Yes, I have one, it's not ready yet though (I'm making it not target specific, it seems ideal for torturing). Segher
Re: [PATCH 1/2] simplify-rtx: The truncation of an IOR can have all bits set (PR81423)
On 07/18/2017 01:36 PM, Segher Boessenkool wrote: > ... if it is an IOR with a constant with all bits set in the mode > that is truncated to, for example. Handle that case. > > With this patch the problematic situation for the PR81423 testcase > isn't even reached; but the next patch fixes that anyway. > > Bootstrapped and tested on powerpc64-linux {-m32,-m64} and on > x86_64-linux. Is this okay for trunk? > > > Segher > > > 2017-07-18 Segher Boessenkool > > PR rtl-optimization/81423 > * simplify-rtx.c (simplify_truncation): Handle truncating an IOR > with a constant that is -1 in the truncated to mode. OK. A testcase would be advisable :-) jeff