[PATCH] Fix PR64822: incorrect folding of bitfield in union on big endian targets

[PATCH] Fix PR64822: incorrect folding of bitfield in union on big endian targets

[Thomas Preud'homme]

Hi,

Incorrect folding happen when a bitfield in a union is accessed and the union was initialized via another field. This is a reminiscence of GCC32RM-325 due to sccvn not passing the right size to fold_ctor_reference. sccvn uses TYPE_SIZE which gives the size of the container integer for a bitfield instead of TYPE_PRECISION that gives the actual size occupied by the bitfield. This patch fixes that and modify bitfld-6 so that it catch this issue in addition to the old one.

ChangeLog entries are as follows:

*** gcc/ChangeLog ***

2015-01-28  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * tree-ssa-sccvn.c (fully_constant_vn_reference_p): Use TYPE_PRECISION
        to compute size of referenced value in the constant case.

*** gcc/testsuite/ChangeLog ***

2015-01-28  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * gcc.c-torture/execute/bitfld-6.c: Use 24 bits for bitfield b.  Adapt
        expected values accordingly.

diff --git a/gcc/testsuite/gcc.c-torture/execute/bitfld-6.c b/gcc/testsuite/gcc.c-torture/execute/bitfld-6.c
index 50927dc..e9a61df 100644
--- a/gcc/testsuite/gcc.c-torture/execute/bitfld-6.c
+++ b/gcc/testsuite/gcc.c-torture/execute/bitfld-6.c
@@ -1,7 +1,7 @@
 union U
 {
   const int a;
-  unsigned b : 20;
+  unsigned b : 24;
 };
 
 static union U u = { 0x12345678 };
@@ -14,9 +14,9 @@ main (void)
 {
 #ifdef __BYTE_ORDER__
 #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
-  return u.b - 0x45678;
+  return u.b - 0x345678;
 #else
-  return u.b - 0x12345;
+  return u.b - 0x123456;
 #endif
 #endif
   return 0;
diff --git a/gcc/tree-ssa-sccvn.c b/gcc/tree-ssa-sccvn.c
index 25c67d0..0f1299a 100644
--- a/gcc/tree-ssa-sccvn.c
+++ b/gcc/tree-ssa-sccvn.c
@@ -1352,7 +1352,7 @@ fully_constant_vn_reference_p (vn_reference_t ref)
 	       || TYPE_PRECISION (ref->type) % BITS_PER_UNIT == 0))
     {
       HOST_WIDE_INT off = 0;
-      HOST_WIDE_INT size = tree_to_shwi (TYPE_SIZE (ref->type));
+      HOST_WIDE_INT size = TYPE_PRECISION (ref->type);
       if (size % BITS_PER_UNIT != 0
 	  || size > MAX_BITSIZE_MODE_ANY_MODE)
 	return NULL_TREE;

Tested by doing a native x86_64 bootstrap + testsuite run and building a arm-none-eabi cross-compiler + testsuite run (on QEMU, compiling for Cortex-M3) without any regression.

Is this ok for trunk?

Best regards,

Thomas

Re: [PATCH] Fix PR64822: incorrect folding of bitfield in union on big endian targets

[Jakub Jelinek]

On Thu, Jan 29, 2015 at 06:27:36PM +0800, Thomas Preud'homme wrote:
> Incorrect folding happen when a bitfield in a union is accessed and the union was initialized via another field. This is a reminiscence of GCC32RM-325 due to sccvn not passing the right size to fold_ctor_reference. sccvn uses TYPE_SIZE which gives the size of the container integer for a bitfield instead of TYPE_PRECISION that gives the actual size occupied by the bitfield. This patch fixes that and modify bitfld-6 so that it catch this issue in addition to the old one.
> 
> ChangeLog entries are as follows:
> 
> *** gcc/ChangeLog ***
> 
> 2015-01-28  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 

You should mention
	PR middle-end/62103
I'll defer review of the sccvn change to Richard.

>         * tree-ssa-sccvn.c (fully_constant_vn_reference_p): Use TYPE_PRECISION
>         to compute size of referenced value in the constant case.
> 
> *** gcc/testsuite/ChangeLog ***
> 
> 2015-01-28  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * gcc.c-torture/execute/bitfld-6.c: Use 24 bits for bitfield b.  Adapt
>         expected values accordingly.

IMHO if the old testcase wasn't incorrect, you'd better add a new testcase
instead of modifying existing one.

	Jakub

Re: [PATCH] Fix PR64822: incorrect folding of bitfield in union on big endian targets

[Richard Biener]

On Thu, Jan 29, 2015 at 11:39 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Jan 29, 2015 at 06:27:36PM +0800, Thomas Preud'homme wrote:
>> Incorrect folding happen when a bitfield in a union is accessed and the union was initialized via another field. This is a reminiscence of GCC32RM-325 due to sccvn not passing the right size to fold_ctor_reference. sccvn uses TYPE_SIZE which gives the size of the container integer for a bitfield instead of TYPE_PRECISION that gives the actual size occupied by the bitfield. This patch fixes that and modify bitfld-6 so that it catch this issue in addition to the old one.
>>
>> ChangeLog entries are as follows:
>>
>> *** gcc/ChangeLog ***
>>
>> 2015-01-28  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>
> You should mention
>         PR middle-end/62103
> I'll defer review of the sccvn change to Richard.
>
>>         * tree-ssa-sccvn.c (fully_constant_vn_reference_p): Use TYPE_PRECISION
>>         to compute size of referenced value in the constant case.
>>
>> *** gcc/testsuite/ChangeLog ***
>>
>> 2015-01-28  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>>         * gcc.c-torture/execute/bitfld-6.c: Use 24 bits for bitfield b.  Adapt
>>         expected values accordingly.
>
> IMHO if the old testcase wasn't incorrect, you'd better add a new testcase
> instead of modifying existing one.

Yeah - please add a new testcase.  The patch is otherwise ok.

Thanks,
Richard.

>         Jakub

RE: [PATCH] Fix PR64822: incorrect folding of bitfield in union on big endian targets

[Thomas Preud'homme]

> From: Jakub Jelinek [mailto:jakub@redhat.com]
> Sent: Thursday, January 29, 2015 6:39 PM
> 

> You should mention
> 	PR middle-end/62103

Right, please find the new ChangeLog entries below:

2015-01-30  Thomas Preud'homme  <thomas.preudhomme@arm.com>

	PR middle-end/62103
	* tree-ssa-sccvn.c (fully_constant_vn_reference_p): Use TYPE_PRECISION
	to compute size of referenced value in the constant case.

2015-01-30  Thomas Preud'homme  <thomas.preudhomme@arm.com>

	PR middle-end/62103
	* gcc.c-torture/execute/bitfld-7.c: New test adapted from bitfld-6.c
	to use 24 bits for bitfield b.

> > 2015-01-28  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> >
> >         * gcc.c-torture/execute/bitfld-6.c: Use 24 bits for bitfield b.  Adapt
> >         expected values accordingly.
> 
> IMHO if the old testcase wasn't incorrect, you'd better add a new
> testcase
> instead of modifying existing one.

Alright, here you are. I changed the existing testcase because the new setup felt as
a superset. Both setup goes into the code I added back then when I closed PR62103
but the new setup reaches it from SCC value numbering code.

You're right though the two testcases will exercise different code path for the rest
of the compiler and there is value in having both.

diff --git a/gcc/testsuite/gcc.c-torture/execute/bitfld-7.c b/gcc/testsuite/gcc.c-torture/execute/bitfld-7.c
new file mode 100644
index 0000000..e9a61df
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/bitfld-7.c
@@ -0,0 +1,23 @@
+union U
+{
+  const int a;
+  unsigned b : 24;
+};
+
+static union U u = { 0x12345678 };
+
+/* Constant folding used to fail to account for endianness when folding a
+   union.  */
+
+int
+main (void)
+{
+#ifdef __BYTE_ORDER__
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+  return u.b - 0x345678;
+#else
+  return u.b - 0x123456;
+#endif
+#endif
+  return 0;
+}
diff --git a/gcc/tree-ssa-sccvn.c b/gcc/tree-ssa-sccvn.c
index 25c67d0..0f1299a 100644
--- a/gcc/tree-ssa-sccvn.c
+++ b/gcc/tree-ssa-sccvn.c
@@ -1352,7 +1352,7 @@ fully_constant_vn_reference_p (vn_reference_t ref)
 	       || TYPE_PRECISION (ref->type) % BITS_PER_UNIT == 0))
     {
       HOST_WIDE_INT off = 0;
-      HOST_WIDE_INT size = tree_to_shwi (TYPE_SIZE (ref->type));
+      HOST_WIDE_INT size = TYPE_PRECISION (ref->type);
       if (size % BITS_PER_UNIT != 0
 	  || size > MAX_BITSIZE_MODE_ANY_MODE)
 	return NULL_TREE;

Is this ok for trunk?

Best regards,

Thomas

Re: [PATCH] Fix PR64822: incorrect folding of bitfield in union on big endian targets

[Jakub Jelinek]

On Wed, Feb 04, 2015 at 10:45:11AM +0800, Thomas Preud'homme wrote:
> 2015-01-30  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
> 	PR middle-end/62103
> 	* tree-ssa-sccvn.c (fully_constant_vn_reference_p): Use TYPE_PRECISION
> 	to compute size of referenced value in the constant case.
> 
> 2015-01-30  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
> 	PR middle-end/62103
> 	* gcc.c-torture/execute/bitfld-7.c: New test adapted from bitfld-6.c
> 	to use 24 bits for bitfield b.

Richard already acked it with the new testcase, so yes, this is ok for the
trunk (just use today's date).

	Jakub

RE: [PATCH] Fix PR64822: incorrect folding of bitfield in union on big endian targets

[Thomas Preud'homme]

> From: Jakub Jelinek [mailto:jakub@redhat.com]
> Sent: Wednesday, February 04, 2015 3:54 PM
> 
> Richard already acked it with the new testcase, so yes, this is ok for the
> trunk (just use today's date).

Oups my bad, I forgot he acked it.

Thanks and best regards.

Thomas