[PATCH] Fix PR61375: cancel bswap optimization when value doesn't fit in a HOST_WIDE_INT

[PATCH] Fix PR61375: cancel bswap optimization when value doesn't fit in a HOST_WIDE_INT

[Thomas Preud'homme]

When analyzing a bitwise AND with a constant as part of a bitwise OR,
the bswap pass stores the constant in a int64_t variable without checking
if it fits. As a result, we get ICE when the constant is an __int128 value.
This affects GCC trunk but also GCC 4.9 and 4.8 (and possibly earlier
version as well).


ChangeLog are changed as follows:

*** gcc/ChangeLog ***

2014-06-05  Thomas Preud'homme  <thomas.preudhomme@arm.com>

	PR tree-optimization/61375
	* tree-ssa-math-opts.c (init_symbolic_number): Cancel optimization if
	symbolic number cannot be represented in an unsigned HOST_WIDE_INT.
	(find_bswap_or_nop_1): Likewise.

*** gcc/testsuite/ChangeLog ***

2014-06-05  Thomas Preud'homme  <thomas.preudhomme@arm.com>

	PR tree-optimization/61375
	* gcc.c-torture/execute/pr61375-1.c: New test.


diff --git a/gcc/testsuite/gcc.c-torture/execute/pr61375.c b/gcc/testsuite/gcc.c-torture/execute/pr61375.c
new file mode 100644
index 0000000..58df57a
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr61375.c
@@ -0,0 +1,34 @@
+#ifdef __UINT64_TYPE__
+typedef __UINT64_TYPE__ uint64_t;
+#else
+typedef unsigned long long uint64_t;
+#endif
+
+#ifndef __SIZEOF_INT128__
+#define __int128 long long
+#endif
+
+/* Some version of bswap optimization would ICE when analyzing a mask constant
+   too big for an HOST_WIDE_INT (PR210931).  */
+
+__attribute__ ((noinline, noclone)) uint64_t
+uint128_central_bitsi_ior (unsigned __int128 in1, uint64_t in2)
+{
+  __int128 mask = (__int128)0xffff << 56;
+  return ((in1 & mask) >> 56) | in2;
+}
+
+int main(int argc)
+{
+  __int128 in = 1;
+#ifdef __SIZEOF_INT128__
+  in <<= 64;
+#endif
+  if (sizeof (uint64_t) * __CHAR_BIT__ != 64)
+    return 0;
+  if (sizeof (unsigned __int128) * __CHAR_BIT__ != 128)
+    return 0;
+  if (uint128_central_bitsi_ior (in, 2) != 0x102)
+    __builtin_abort ();
+  return 0;
+}
diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
index 658b341..95b3f25 100644
--- a/gcc/tree-ssa-math-opts.c
+++ b/gcc/tree-ssa-math-opts.c
@@ -1717,6 +1717,8 @@ init_symbolic_number (struct symbolic_number *n, tree src)
   if (n->size % BITS_PER_UNIT != 0)
     return false;
   n->size /= BITS_PER_UNIT;
+  if (n->size > (int)sizeof (unsigned HOST_WIDE_INT))
+    return false;
   n->range = n->size;
   n->n = CMPNOP;
 
@@ -1883,6 +1885,8 @@ find_bswap_or_nop_1 (gimple stmt, struct symbolic_number *n, int limit)
 	    type_size = TYPE_PRECISION (gimple_expr_type (stmt));
 	    if (type_size % BITS_PER_UNIT != 0)
 	      return NULL_TREE;
+	    if (type_size > (int)sizeof (unsigned HOST_WIDE_INT) * 8)
+	      return NULL_TREE;
 
 	    if (type_size / BITS_PER_UNIT < (int)(sizeof (int64_t)))
 	      {

Is this OK for trunk? What about backports for 4.8 and 4.9? Would a
reworked patch for these versions be accepted? The change would
be trivial: the code in init_symbolic_number now was moved from
some other place.

Best regards,

Thomas

Re: [PATCH] Fix PR61375: cancel bswap optimization when value doesn't fit in a HOST_WIDE_INT

[Richard Biener]

On Tue, Jun 10, 2014 at 4:30 AM, Thomas Preud'homme
<thomas.preudhomme@arm.com> wrote:
> When analyzing a bitwise AND with a constant as part of a bitwise OR,
> the bswap pass stores the constant in a int64_t variable without checking
> if it fits. As a result, we get ICE when the constant is an __int128 value.
> This affects GCC trunk but also GCC 4.9 and 4.8 (and possibly earlier
> version as well).
>
>
> ChangeLog are changed as follows:
>
> *** gcc/ChangeLog ***
>
> 2014-06-05  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>         PR tree-optimization/61375
>         * tree-ssa-math-opts.c (init_symbolic_number): Cancel optimization if
>         symbolic number cannot be represented in an unsigned HOST_WIDE_INT.
>         (find_bswap_or_nop_1): Likewise.
>
> *** gcc/testsuite/ChangeLog ***
>
> 2014-06-05  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>         PR tree-optimization/61375
>         * gcc.c-torture/execute/pr61375-1.c: New test.
>
>
> diff --git a/gcc/testsuite/gcc.c-torture/execute/pr61375.c b/gcc/testsuite/gcc.c-torture/execute/pr61375.c
> new file mode 100644
> index 0000000..58df57a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/execute/pr61375.c
> @@ -0,0 +1,34 @@
> +#ifdef __UINT64_TYPE__
> +typedef __UINT64_TYPE__ uint64_t;
> +#else
> +typedef unsigned long long uint64_t;
> +#endif
> +
> +#ifndef __SIZEOF_INT128__
> +#define __int128 long long
> +#endif
> +
> +/* Some version of bswap optimization would ICE when analyzing a mask constant
> +   too big for an HOST_WIDE_INT (PR210931).  */
> +
> +__attribute__ ((noinline, noclone)) uint64_t
> +uint128_central_bitsi_ior (unsigned __int128 in1, uint64_t in2)
> +{
> +  __int128 mask = (__int128)0xffff << 56;
> +  return ((in1 & mask) >> 56) | in2;
> +}
> +
> +int main(int argc)
> +{
> +  __int128 in = 1;
> +#ifdef __SIZEOF_INT128__
> +  in <<= 64;
> +#endif
> +  if (sizeof (uint64_t) * __CHAR_BIT__ != 64)
> +    return 0;
> +  if (sizeof (unsigned __int128) * __CHAR_BIT__ != 128)
> +    return 0;
> +  if (uint128_central_bitsi_ior (in, 2) != 0x102)
> +    __builtin_abort ();
> +  return 0;
> +}
> diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
> index 658b341..95b3f25 100644
> --- a/gcc/tree-ssa-math-opts.c
> +++ b/gcc/tree-ssa-math-opts.c
> @@ -1717,6 +1717,8 @@ init_symbolic_number (struct symbolic_number *n, tree src)
>    if (n->size % BITS_PER_UNIT != 0)
>      return false;
>    n->size /= BITS_PER_UNIT;
> +  if (n->size > (int)sizeof (unsigned HOST_WIDE_INT))

this should be sizeof (uint64_t) on the trunk and sizeof (unsigned
HOST_WIDEST_INT) on branches.

> +    return false;
>    n->range = n->size;
>    n->n = CMPNOP;
>
> @@ -1883,6 +1885,8 @@ find_bswap_or_nop_1 (gimple stmt, struct symbolic_number *n, int limit)
>             type_size = TYPE_PRECISION (gimple_expr_type (stmt));
>             if (type_size % BITS_PER_UNIT != 0)
>               return NULL_TREE;
> +           if (type_size > (int)sizeof (unsigned HOST_WIDE_INT) * 8)
> +             return NULL_TREE;

Likewise.

>             if (type_size / BITS_PER_UNIT < (int)(sizeof (int64_t)))
>               {
>
> Is this OK for trunk?

Ok for trunk with using uint64_t.

 What about backports for 4.8 and 4.9? Would a
> reworked patch for these versions be accepted? The change would
> be trivial: the code in init_symbolic_number now was moved from
> some other place.

Backports are welcome - please post a patch.

Thanks,
Richard.

> Best regards,
>
> Thomas
>
>

RE: [PATCH] Fix PR61375: cancel bswap optimization when value doesn't fit in a HOST_WIDE_INT

[Thomas Preud'homme]

> From: Richard Biener [mailto:richard.guenther@gmail.com]
> Sent: Tuesday, June 10, 2014 5:05 PM
> 
> Backports are welcome - please post a patch.
> 

Sorry for the delay. Here you are:

diff --git a/gcc/testsuite/gcc.c-torture/execute/pr61375.c b/gcc/testsuite/gcc.c-torture/execute/pr61375.c
new file mode 100644
index 0000000..d3b54a8
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr61375.c
@@ -0,0 +1,35 @@
+#ifdef __UINT64_TYPE__
+typedef __UINT64_TYPE__ uint64_t;
+#else
+typedef unsigned long long uint64_t;
+#endif
+
+#ifndef __SIZEOF_INT128__
+#define __int128 long long
+#endif
+
+/* Some version of bswap optimization would ICE when analyzing a mask constant
+   too big for an HOST_WIDE_INT (PR61375).  */
+
+__attribute__ ((noinline, noclone)) uint64_t
+uint128_central_bitsi_ior (unsigned __int128 in1, uint64_t in2)
+{
+  __int128 mask = (__int128)0xffff << 56;
+  return ((in1 & mask) >> 56) | in2;
+}
+
+int
+main (int argc)
+{
+  __int128 in = 1;
+#ifdef __SIZEOF_INT128__
+  in <<= 64;
+#endif
+  if (sizeof (uint64_t) * __CHAR_BIT__ != 64)
+    return 0;
+  if (sizeof (unsigned __int128) * __CHAR_BIT__ != 128)
+    return 0;
+  if (uint128_central_bitsi_ior (in, 2) != 0x102)
+    __builtin_abort ();
+  return 0;
+}
diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
index 9ff857c..9d64205 100644
--- a/gcc/tree-ssa-math-opts.c
+++ b/gcc/tree-ssa-math-opts.c
@@ -1741,6 +1741,8 @@ find_bswap_1 (gimple stmt, struct symbolic_number *n, int limit)
 	  if (n->size % BITS_PER_UNIT != 0)
 	    return NULL_TREE;
 	  n->size /= BITS_PER_UNIT;
+	  if (n->size > (int)sizeof (unsigned HOST_WIDEST_INT))
+	    return NULL_TREE;
 	  n->n = (sizeof (HOST_WIDEST_INT) < 8 ? 0 :
 		  (unsigned HOST_WIDEST_INT)0x08070605 << 32 | 0x04030201);
 
@@ -1781,6 +1783,8 @@ find_bswap_1 (gimple stmt, struct symbolic_number *n, int limit)
 	    type_size = TYPE_PRECISION (gimple_expr_type (stmt));
 	    if (type_size % BITS_PER_UNIT != 0)
 	      return NULL_TREE;
+	    if (type_size > (int)HOST_BITS_PER_WIDEST_INT)
+	      return NULL_TREE;
 
 	    if (type_size / BITS_PER_UNIT < (int)(sizeof (HOST_WIDEST_INT)))
 	      {

Ok for GCC 4.8 and GCC 4.9 branches?

Best regards,

Thomas

Re: [PATCH] Fix PR61375: cancel bswap optimization when value doesn't fit in a HOST_WIDE_INT

[Richard Biener]

On Fri, Jun 20, 2014 at 12:41 PM, Thomas Preud'homme
<thomas.preudhomme@arm.com> wrote:
>> From: Richard Biener [mailto:richard.guenther@gmail.com]
>> Sent: Tuesday, June 10, 2014 5:05 PM
>>
>> Backports are welcome - please post a patch.
>>
>
> Sorry for the delay. Here you are:
>
> diff --git a/gcc/testsuite/gcc.c-torture/execute/pr61375.c b/gcc/testsuite/gcc.c-torture/execute/pr61375.c
> new file mode 100644
> index 0000000..d3b54a8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/execute/pr61375.c
> @@ -0,0 +1,35 @@
> +#ifdef __UINT64_TYPE__
> +typedef __UINT64_TYPE__ uint64_t;
> +#else
> +typedef unsigned long long uint64_t;
> +#endif
> +
> +#ifndef __SIZEOF_INT128__
> +#define __int128 long long
> +#endif
> +
> +/* Some version of bswap optimization would ICE when analyzing a mask constant
> +   too big for an HOST_WIDE_INT (PR61375).  */
> +
> +__attribute__ ((noinline, noclone)) uint64_t
> +uint128_central_bitsi_ior (unsigned __int128 in1, uint64_t in2)
> +{
> +  __int128 mask = (__int128)0xffff << 56;
> +  return ((in1 & mask) >> 56) | in2;
> +}
> +
> +int
> +main (int argc)
> +{
> +  __int128 in = 1;
> +#ifdef __SIZEOF_INT128__
> +  in <<= 64;
> +#endif
> +  if (sizeof (uint64_t) * __CHAR_BIT__ != 64)
> +    return 0;
> +  if (sizeof (unsigned __int128) * __CHAR_BIT__ != 128)
> +    return 0;
> +  if (uint128_central_bitsi_ior (in, 2) != 0x102)
> +    __builtin_abort ();
> +  return 0;
> +}
> diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
> index 9ff857c..9d64205 100644
> --- a/gcc/tree-ssa-math-opts.c
> +++ b/gcc/tree-ssa-math-opts.c
> @@ -1741,6 +1741,8 @@ find_bswap_1 (gimple stmt, struct symbolic_number *n, int limit)
>           if (n->size % BITS_PER_UNIT != 0)
>             return NULL_TREE;
>           n->size /= BITS_PER_UNIT;
> +         if (n->size > (int)sizeof (unsigned HOST_WIDEST_INT))
> +           return NULL_TREE;
>           n->n = (sizeof (HOST_WIDEST_INT) < 8 ? 0 :
>                   (unsigned HOST_WIDEST_INT)0x08070605 << 32 | 0x04030201);
>
> @@ -1781,6 +1783,8 @@ find_bswap_1 (gimple stmt, struct symbolic_number *n, int limit)
>             type_size = TYPE_PRECISION (gimple_expr_type (stmt));
>             if (type_size % BITS_PER_UNIT != 0)
>               return NULL_TREE;
> +           if (type_size > (int)HOST_BITS_PER_WIDEST_INT)
> +             return NULL_TREE;
>
>             if (type_size / BITS_PER_UNIT < (int)(sizeof (HOST_WIDEST_INT)))
>               {
>
> Ok for GCC 4.8 and GCC 4.9 branches?

Ok.

Thanks,
Richard.

> Best regards,
>
> Thomas
>
>

Re: [PATCH] Fix PR61375: cancel bswap optimization when value doesn't fit in a HOST_WIDE_INT

[Jakub Jelinek]

On Mon, Jun 23, 2014 at 10:18:16AM +0200, Richard Biener wrote:
> > --- a/gcc/tree-ssa-math-opts.c
> > +++ b/gcc/tree-ssa-math-opts.c
> > @@ -1741,6 +1741,8 @@ find_bswap_1 (gimple stmt, struct symbolic_number *n, int limit)
> >           if (n->size % BITS_PER_UNIT != 0)
> >             return NULL_TREE;
> >           n->size /= BITS_PER_UNIT;
> > +         if (n->size > (int)sizeof (unsigned HOST_WIDEST_INT))
> > +           return NULL_TREE;

This looks wrong, while the bswap pass is guarded with BITS_PER_UNIT == 8
check (i.e. target), you don't know of HOST_BITS_PER_CHAR is 8.
I'd move the test before the division by BITS_PER_UNIT, and compare
against HOST_BITS_PER_WIDEST_INT.

> >           n->n = (sizeof (HOST_WIDEST_INT) < 8 ? 0 :
> >                   (unsigned HOST_WIDEST_INT)0x08070605 << 32 | 0x04030201);
> >
> > @@ -1781,6 +1783,8 @@ find_bswap_1 (gimple stmt, struct symbolic_number *n, int limit)
> >             type_size = TYPE_PRECISION (gimple_expr_type (stmt));
> >             if (type_size % BITS_PER_UNIT != 0)
> >               return NULL_TREE;
> > +           if (type_size > (int)HOST_BITS_PER_WIDEST_INT)
> > +             return NULL_TREE;
> >
> >             if (type_size / BITS_PER_UNIT < (int)(sizeof (HOST_WIDEST_INT)))
> >               {

Similarly here.

BTW, the formatting is wrong too, the (int) cast should be followed by space.

	Jakub

RE: [PATCH] Fix PR61375: cancel bswap optimization when value doesn't fit in a HOST_WIDE_INT

[Thomas Preud'homme]

> From: Jakub Jelinek [mailto:jakub@redhat.com]
> Sent: Monday, June 23, 2014 4:37 PM
> 
> On Mon, Jun 23, 2014 at 10:18:16AM +0200, Richard Biener wrote:
> > > --- a/gcc/tree-ssa-math-opts.c
> > > +++ b/gcc/tree-ssa-math-opts.c
> > > @@ -1741,6 +1741,8 @@ find_bswap_1 (gimple stmt, struct
> symbolic_number *n, int limit)
> > >           if (n->size % BITS_PER_UNIT != 0)
> > >             return NULL_TREE;
> > >           n->size /= BITS_PER_UNIT;
> > > +         if (n->size > (int)sizeof (unsigned HOST_WIDEST_INT))
> > > +           return NULL_TREE;
> 
> This looks wrong, while the bswap pass is guarded with BITS_PER_UNIT == 8
> check (i.e. target), you don't know of HOST_BITS_PER_CHAR is 8.
> I'd move the test before the division by BITS_PER_UNIT, and compare
> against HOST_BITS_PER_WIDEST_INT.

I may misunderstand you but I don't think there is a problem here because we
just check if we can create a value on the host with as many bytes as the value
on the target. The value on the host is different, with each byte being a
number from 1 to SIZE, SIZE being the number of bytes on the target. So this
would fail only if the target value has so many bytes that this number of byte
cannot be represented in a HOST_WIDEST_INT.

> 
> > >           n->n = (sizeof (HOST_WIDEST_INT) < 8 ? 0 :
> > >                   (unsigned HOST_WIDEST_INT)0x08070605 << 32 | 0x04030201);
> > >
> > > @@ -1781,6 +1783,8 @@ find_bswap_1 (gimple stmt, struct
> symbolic_number *n, int limit)
> > >             type_size = TYPE_PRECISION (gimple_expr_type (stmt));
> > >             if (type_size % BITS_PER_UNIT != 0)
> > >               return NULL_TREE;
> > > +           if (type_size > (int)HOST_BITS_PER_WIDEST_INT)
> > > +             return NULL_TREE;
> > >
> > >             if (type_size / BITS_PER_UNIT < (int)(sizeof (HOST_WIDEST_INT)))
> > >               {
> 
> Similarly here.

I agree that here the test is not correct as we look at the number of bits on the
host which should be enough to count the number of byte on the target. To
reflect better the intent we should first compute the number of byte that
type_size forms and then compare to the size in byte of HOST_WIDEST_INT.

I'll rework the patch in this directly.

> 
> BTW, the formatting is wrong too, the (int) cast should be followed by space.

Right, but note that I merely followed the current style in this file. There are
many more occurences of this style mistake in this file. Do you want me to
fix this one anyway?

Best regards,

Thomas

Re: [PATCH] Fix PR61375: cancel bswap optimization when value doesn't fit in a HOST_WIDE_INT

[Jakub Jelinek]

On Mon, Jun 23, 2014 at 04:50:49PM +0800, Thomas Preud'homme wrote:
> > Sent: Monday, June 23, 2014 4:37 PM
> > 
> > On Mon, Jun 23, 2014 at 10:18:16AM +0200, Richard Biener wrote:
> > > > --- a/gcc/tree-ssa-math-opts.c
> > > > +++ b/gcc/tree-ssa-math-opts.c
> > > > @@ -1741,6 +1741,8 @@ find_bswap_1 (gimple stmt, struct
> > symbolic_number *n, int limit)
> > > >           if (n->size % BITS_PER_UNIT != 0)
> > > >             return NULL_TREE;
> > > >           n->size /= BITS_PER_UNIT;
> > > > +         if (n->size > (int)sizeof (unsigned HOST_WIDEST_INT))
> > > > +           return NULL_TREE;
> > 
> > This looks wrong, while the bswap pass is guarded with BITS_PER_UNIT == 8
> > check (i.e. target), you don't know of HOST_BITS_PER_CHAR is 8.
> > I'd move the test before the division by BITS_PER_UNIT, and compare
> > against HOST_BITS_PER_WIDEST_INT.
> 
> I may misunderstand you but I don't think there is a problem here because we
> just check if we can create a value on the host with as many bytes as the value
> on the target. The value on the host is different, with each byte being a
> number from 1 to SIZE, SIZE being the number of bytes on the target. So this
> would fail only if the target value has so many bytes that this number of byte
> cannot be represented in a HOST_WIDEST_INT.

Host could e.g. in theory have CHAR_BIT 32, while target BITS_PER_UNIT 8
(otherwise bswap pass would give up).  sizeof (unsigned HOST_WIDE_INT) could
very well be 2 in that case.

Anyway, another option is to also not run the bswap pass if CHAR_BIT != 8,
e.g. fold-const.c does something similar when deciding if VIEW_CONVERT_EXPR
of constants can be safely folded.

> > BTW, the formatting is wrong too, the (int) cast should be followed by space.
> 
> Right, but note that I merely followed the current style in this file. There are
> many more occurences of this style mistake in this file. Do you want me to
> fix this one anyway?

Just don't introduce new formatting issues and on lines you touch anyway
also fix formatting issues.

	Jakub

RE: [PATCH] Fix PR61375: cancel bswap optimization when value doesn't fit in a HOST_WIDE_INT

[Thomas Preud'homme]

> From: Jakub Jelinek [mailto:jakub@redhat.com]
> Sent: Monday, June 23, 2014 4:59 PM
> 
> Host could e.g. in theory have CHAR_BIT 32, while target BITS_PER_UNIT 8
> (otherwise bswap pass would give up).  sizeof (unsigned HOST_WIDE_INT)
> could
> very well be 2 in that case.

In this case the pass would skip any value of more than 2 bytes. However although the original comments on struct symbolic_number implies that there is a mapping between host bytes (the bytes of the symbolic number) and target bytes, it isn't the case since do_shift_rotate () shift the symbolic number by quantity of BYTES_PER_UNIT instead of CHAR_BIT. Also there is quite a few 8 here and there. Although not a problem in practice, the mix of 8 and BITS_PER_UNIT does not look very good. I guess a quick review would be in order. Of course, with regards to the backport the mix of 8 and BITS_PER_UNIT should be left as is and only confusion about how to represent a target value into a host type should be fixed if any.

I'll come back to you whenever this is done.

Best regards,

Thomas

RE: [PATCH] Fix PR61375: cancel bswap optimization when value doesn't fit in a HOST_WIDE_INT

[Thomas Preud'homme]

> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Thomas Preud'homme
> 
> However
> although the original comments on struct symbolic_number implies that
> there is a mapping between host bytes (the bytes of the symbolic number)
> and target bytes, it isn't the case since do_shift_rotate () shift the symbolic
> number by quantity of BYTES_PER_UNIT instead of CHAR_BIT.

My bad, the comment can be understood both ways.

Best regards,

Thomas

RE: [PATCH] Fix PR61375: cancel bswap optimization when value doesn't fit in a HOST_WIDE_INT

[Thomas Preud'homme]

Ok, what about the following patch and associated ChangeLog entries?

2014-06-24  Thomas Preud'homme  <thomas.preudhomme@arm.com>

	PR tree-optimization/61375
	* tree-ssa-math-opts.c (find_bswap_or_nop_1): Cancel optimization if
	symbolic number cannot be represented in an unsigned HOST_WIDE_INT.
	(execute_optimize_bswap): Cancel optimization if CHAR_BIT != 8.

2014-06-24  Thomas Preud'homme  <thomas.preudhomme@arm.com>

	PR tree-optimization/61375
	* gcc.c-torture/execute/pr61375-1.c: New test.


diff --git a/gcc/testsuite/gcc.c-torture/execute/pr61375.c b/gcc/testsuite/gcc.c-torture/execute/pr61375.c
new file mode 100644
index 0000000..6fb4693
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr61375.c
@@ -0,0 +1,35 @@
+#ifdef __UINT64_TYPE__
+typedef __UINT64_TYPE__ uint64_t;
+#else
+typedef unsigned long long uint64_t;
+#endif
+
+#ifndef __SIZEOF_INT128__
+#define __int128 long long
+#endif
+
+/* Some version of bswap optimization would ICE when analyzing a mask constant
+   too big for an HOST_WIDE_INT (PR61375).  */
+
+__attribute__ ((noinline, noclone)) uint64_t
+uint128_central_bitsi_ior (unsigned __int128 in1, uint64_t in2)
+{
+  __int128 mask = (__int128)0xffff << 56;
+  return ((in1 & mask) >> 56) | in2;
+}
+
+int
+main (int argc)
+{
+  __int128 in = 1;
+#ifdef __SIZEOF_INT128__
+  in <<= 64;
+#endif
+  if (sizeof (uint64_t) * __CHAR_BIT__ != 64)
+    return 0;
+  if (sizeof (unsigned __int128) * __CHAR_BIT__ != 128)
+    return 0;
+  if (uint128_central_bitsi_ior (in, 2) != 0x102)
+    __builtin_abort ();
+  return 0;
+}
diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
index 9ff857c..045bf48 100644
--- a/gcc/tree-ssa-math-opts.c
+++ b/gcc/tree-ssa-math-opts.c
@@ -1740,6 +1740,8 @@ find_bswap_1 (gimple stmt, struct symbolic_number *n, int limit)
 	  n->size = TYPE_PRECISION (TREE_TYPE (rhs1));
 	  if (n->size % BITS_PER_UNIT != 0)
 	    return NULL_TREE;
+	  if (n->size > HOST_BITS_PER_WIDEST_INT)
+	    return NULL_TREE;
 	  n->size /= BITS_PER_UNIT;
 	  n->n = (sizeof (HOST_WIDEST_INT) < 8 ? 0 :
 		  (unsigned HOST_WIDEST_INT)0x08070605 << 32 | 0x04030201);
@@ -1781,6 +1783,8 @@ find_bswap_1 (gimple stmt, struct symbolic_number *n, int limit)
 	    type_size = TYPE_PRECISION (gimple_expr_type (stmt));
 	    if (type_size % BITS_PER_UNIT != 0)
 	      return NULL_TREE;
+	    if (type_size > (int) HOST_BITS_PER_WIDEST_INT)
+	      return NULL_TREE;
 
 	    if (type_size / BITS_PER_UNIT < (int)(sizeof (HOST_WIDEST_INT)))
 	      {
@@ -1911,7 +1915,7 @@ execute_optimize_bswap (void)
   bool changed = false;
   tree bswap16_type = NULL_TREE, bswap32_type = NULL_TREE, bswap64_type = NULL_TREE;
 
-  if (BITS_PER_UNIT != 8)
+  if (BITS_PER_UNIT != 8 || CHAR_BIT != 8)
     return 0;
 
   if (sizeof (HOST_WIDEST_INT) < 8)

Is this ok for 4.8 and 4.9 branches?

Best regards,

Thomas

RE: [PATCH] Fix PR61375: cancel bswap optimization when value doesn't fit in a HOST_WIDE_INT

[Thomas Preud'homme]

Ping?

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Thomas Preud'homme
> Sent: Thursday, June 26, 2014 9:11 AM
> To: 'Jakub Jelinek'
> Cc: Richard Biener; GCC Patches
> Subject: RE: [PATCH] Fix PR61375: cancel bswap optimization when value
> doesn't fit in a HOST_WIDE_INT
> 
> Ok, what about the following patch and associated ChangeLog entries?
> 
> 2014-06-24  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
> 	PR tree-optimization/61375
> 	* tree-ssa-math-opts.c (find_bswap_or_nop_1): Cancel optimization
> if
> 	symbolic number cannot be represented in an unsigned
> HOST_WIDE_INT.
> 	(execute_optimize_bswap): Cancel optimization if CHAR_BIT != 8.
> 
> 2014-06-24  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
> 	PR tree-optimization/61375
> 	* gcc.c-torture/execute/pr61375-1.c: New test.
> 
> 
> diff --git a/gcc/testsuite/gcc.c-torture/execute/pr61375.c
> b/gcc/testsuite/gcc.c-torture/execute/pr61375.c
> new file mode 100644
> index 0000000..6fb4693
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/execute/pr61375.c
> @@ -0,0 +1,35 @@
> +#ifdef __UINT64_TYPE__
> +typedef __UINT64_TYPE__ uint64_t;
> +#else
> +typedef unsigned long long uint64_t;
> +#endif
> +
> +#ifndef __SIZEOF_INT128__
> +#define __int128 long long
> +#endif
> +
> +/* Some version of bswap optimization would ICE when analyzing a mask
> constant
> +   too big for an HOST_WIDE_INT (PR61375).  */
> +
> +__attribute__ ((noinline, noclone)) uint64_t
> +uint128_central_bitsi_ior (unsigned __int128 in1, uint64_t in2)
> +{
> +  __int128 mask = (__int128)0xffff << 56;
> +  return ((in1 & mask) >> 56) | in2;
> +}
> +
> +int
> +main (int argc)
> +{
> +  __int128 in = 1;
> +#ifdef __SIZEOF_INT128__
> +  in <<= 64;
> +#endif
> +  if (sizeof (uint64_t) * __CHAR_BIT__ != 64)
> +    return 0;
> +  if (sizeof (unsigned __int128) * __CHAR_BIT__ != 128)
> +    return 0;
> +  if (uint128_central_bitsi_ior (in, 2) != 0x102)
> +    __builtin_abort ();
> +  return 0;
> +}
> diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
> index 9ff857c..045bf48 100644
> --- a/gcc/tree-ssa-math-opts.c
> +++ b/gcc/tree-ssa-math-opts.c
> @@ -1740,6 +1740,8 @@ find_bswap_1 (gimple stmt, struct
> symbolic_number *n, int limit)
>  	  n->size = TYPE_PRECISION (TREE_TYPE (rhs1));
>  	  if (n->size % BITS_PER_UNIT != 0)
>  	    return NULL_TREE;
> +	  if (n->size > HOST_BITS_PER_WIDEST_INT)
> +	    return NULL_TREE;
>  	  n->size /= BITS_PER_UNIT;
>  	  n->n = (sizeof (HOST_WIDEST_INT) < 8 ? 0 :
>  		  (unsigned HOST_WIDEST_INT)0x08070605 << 32 |
> 0x04030201);
> @@ -1781,6 +1783,8 @@ find_bswap_1 (gimple stmt, struct
> symbolic_number *n, int limit)
>  	    type_size = TYPE_PRECISION (gimple_expr_type (stmt));
>  	    if (type_size % BITS_PER_UNIT != 0)
>  	      return NULL_TREE;
> +	    if (type_size > (int) HOST_BITS_PER_WIDEST_INT)
> +	      return NULL_TREE;
> 
>  	    if (type_size / BITS_PER_UNIT < (int)(sizeof (HOST_WIDEST_INT)))
>  	      {
> @@ -1911,7 +1915,7 @@ execute_optimize_bswap (void)
>    bool changed = false;
>    tree bswap16_type = NULL_TREE, bswap32_type = NULL_TREE,
> bswap64_type = NULL_TREE;
> 
> -  if (BITS_PER_UNIT != 8)
> +  if (BITS_PER_UNIT != 8 || CHAR_BIT != 8)
>      return 0;
> 
>    if (sizeof (HOST_WIDEST_INT) < 8)
> 
> Is this ok for 4.8 and 4.9 branches?
> 
> Best regards,
> 
> Thomas
> 
> 

RE: [PATCH] Fix PR61375: cancel bswap optimization when value doesn't fit in a HOST_WIDE_INT

[Thomas Preud'homme]

Now that GCC 4.9 branch is opened again and GCC 4.8 branch still open, is the following backported patch ok for both branches?

Best regards,

Thomas

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Thomas Preud'homme
> Sent: Thursday, June 26, 2014 9:11 AM
> To: 'Jakub Jelinek'
> Cc: Richard Biener; GCC Patches
> Subject: RE: [PATCH] Fix PR61375: cancel bswap optimization when value
> doesn't fit in a HOST_WIDE_INT
> 
> Ok, what about the following patch and associated ChangeLog entries?
> 
> 2014-06-24  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
> 	PR tree-optimization/61375
> 	* tree-ssa-math-opts.c (find_bswap_or_nop_1): Cancel optimization
> if
> 	symbolic number cannot be represented in an unsigned
> HOST_WIDE_INT.
> 	(execute_optimize_bswap): Cancel optimization if CHAR_BIT != 8.
> 
> 2014-06-24  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
> 	PR tree-optimization/61375
> 	* gcc.c-torture/execute/pr61375-1.c: New test.
> 
> 
> diff --git a/gcc/testsuite/gcc.c-torture/execute/pr61375.c
> b/gcc/testsuite/gcc.c-torture/execute/pr61375.c
> new file mode 100644
> index 0000000..6fb4693
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/execute/pr61375.c
> @@ -0,0 +1,35 @@
> +#ifdef __UINT64_TYPE__
> +typedef __UINT64_TYPE__ uint64_t;
> +#else
> +typedef unsigned long long uint64_t;
> +#endif
> +
> +#ifndef __SIZEOF_INT128__
> +#define __int128 long long
> +#endif
> +
> +/* Some version of bswap optimization would ICE when analyzing a mask
> constant
> +   too big for an HOST_WIDE_INT (PR61375).  */
> +
> +__attribute__ ((noinline, noclone)) uint64_t
> +uint128_central_bitsi_ior (unsigned __int128 in1, uint64_t in2)
> +{
> +  __int128 mask = (__int128)0xffff << 56;
> +  return ((in1 & mask) >> 56) | in2;
> +}
> +
> +int
> +main (int argc)
> +{
> +  __int128 in = 1;
> +#ifdef __SIZEOF_INT128__
> +  in <<= 64;
> +#endif
> +  if (sizeof (uint64_t) * __CHAR_BIT__ != 64)
> +    return 0;
> +  if (sizeof (unsigned __int128) * __CHAR_BIT__ != 128)
> +    return 0;
> +  if (uint128_central_bitsi_ior (in, 2) != 0x102)
> +    __builtin_abort ();
> +  return 0;
> +}
> diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
> index 9ff857c..045bf48 100644
> --- a/gcc/tree-ssa-math-opts.c
> +++ b/gcc/tree-ssa-math-opts.c
> @@ -1740,6 +1740,8 @@ find_bswap_1 (gimple stmt, struct
> symbolic_number *n, int limit)
>  	  n->size = TYPE_PRECISION (TREE_TYPE (rhs1));
>  	  if (n->size % BITS_PER_UNIT != 0)
>  	    return NULL_TREE;
> +	  if (n->size > HOST_BITS_PER_WIDEST_INT)
> +	    return NULL_TREE;
>  	  n->size /= BITS_PER_UNIT;
>  	  n->n = (sizeof (HOST_WIDEST_INT) < 8 ? 0 :
>  		  (unsigned HOST_WIDEST_INT)0x08070605 << 32 |
> 0x04030201);
> @@ -1781,6 +1783,8 @@ find_bswap_1 (gimple stmt, struct
> symbolic_number *n, int limit)
>  	    type_size = TYPE_PRECISION (gimple_expr_type (stmt));
>  	    if (type_size % BITS_PER_UNIT != 0)
>  	      return NULL_TREE;
> +	    if (type_size > (int) HOST_BITS_PER_WIDEST_INT)
> +	      return NULL_TREE;
> 
>  	    if (type_size / BITS_PER_UNIT < (int)(sizeof (HOST_WIDEST_INT)))
>  	      {
> @@ -1911,7 +1915,7 @@ execute_optimize_bswap (void)
>    bool changed = false;
>    tree bswap16_type = NULL_TREE, bswap32_type = NULL_TREE,
> bswap64_type = NULL_TREE;
> 
> -  if (BITS_PER_UNIT != 8)
> +  if (BITS_PER_UNIT != 8 || CHAR_BIT != 8)
>      return 0;
> 
>    if (sizeof (HOST_WIDEST_INT) < 8)
> 
> Is this ok for 4.8 and 4.9 branches?
> 
> Best regards,
> 
> Thomas
> 
> 

Re: [PATCH] Fix PR61375: cancel bswap optimization when value doesn't fit in a HOST_WIDE_INT

[Jakub Jelinek]

On Thu, Jul 31, 2014 at 02:57:34PM +0800, Thomas Preud'homme wrote:
> Now that GCC 4.9 branch is opened again and GCC 4.8 branch still open, is
> the following backported patch ok for both branches?

Ok.

> > Ok, what about the following patch and associated ChangeLog entries?
> > 
> > 2014-06-24  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> > 
> > 	PR tree-optimization/61375
> > 	* tree-ssa-math-opts.c (find_bswap_or_nop_1): Cancel optimization
> > if
> > 	symbolic number cannot be represented in an unsigned
> > HOST_WIDE_INT.
> > 	(execute_optimize_bswap): Cancel optimization if CHAR_BIT != 8.
> > 
> > 2014-06-24  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> > 
> > 	PR tree-optimization/61375
> > 	* gcc.c-torture/execute/pr61375-1.c: New test.

	Jakub