Re: [PATCH 1/2] simplify-rtx: The truncation of an IOR can have all bits set (PR81423)

[Jeff Law]

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)

[Segher Boessenkool]

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)

[Mike Stump]

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)

[Segher Boessenkool]

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)

[Kyrill Tkachov]

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)

[Segher Boessenkool]

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)

[Jeff Law]

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)

[Segher Boessenkool]

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)

[Segher Boessenkool]

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)

[Jeff Law]

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

[PATCH 1/2] simplify-rtx: The truncation of an IOR can have all bits set (PR81423)

[Segher Boessenkool]

... 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.

---
 gcc/simplify-rtx.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index 3bce329..ef41479 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -857,6 +857,15 @@ simplify_truncation (machine_mode mode, rtx op,
 return simplify_gen_unary (TRUNCATE, mode, XEXP (op, 0),
   GET_MODE (XEXP (op, 0)));
 
+  /* (truncate:A (ior X C)) is (const_int -1) if C is equal to that already,
+ in mode A.  */
+  if (GET_CODE (op) == IOR
+  && SCALAR_INT_MODE_P (mode)
+  && SCALAR_INT_MODE_P (op_mode)
+  && CONST_INT_P (XEXP (op, 1))
+  && trunc_int_for_mode (INTVAL (XEXP (op, 1)), mode) == -1)
+return constm1_rtx;
+
   return NULL_RTX;
 }
 
-- 
1.9.3