Re: ifcvt: Fix bitpos calculation in bitfield lowering [PR107229]

["Andre Vieira (lists)" <andre.simoesdiasvieira@arm.com>]

[-- Attachment #1: Type: text/plain, Size: 1381 bytes --]

Added some extra comments to describe what is going on there.

On 13/10/2022 09:14, Richard Biener wrote:
> On Wed, 12 Oct 2022, Andre Vieira (lists) wrote:
>
>> Hi,
>>
>> The bitposition calculation for the bitfield lowering in loop if conversion
>> was not
>> taking DECL_FIELD_OFFSET into account, which meant that it would result in
>> wrong bitpositions for bitfields that did not end up having representations
>> starting at the beginning of the struct.
>>
>> Bootstrappend and regression tested on aarch64-none-linux-gnu and
>> x86_64-pc-linux-gnu.
> +    {
> +      tree bf_pos = fold_build2 (MULT_EXPR, bitsizetype,
> +                                DECL_FIELD_OFFSET (field_decl),
> +                                build_int_cst (bitsizetype, 8));
> +      bf_pos = fold_build2 (PLUS_EXPR, bitsizetype, bf_pos,
> +                           DECL_FIELD_BIT_OFFSET (field_decl));
> +      tree rep_pos = fold_build2 (MULT_EXPR, bitsizetype,
> +                                 DECL_FIELD_OFFSET (rep_decl),
> +                                 build_int_cst (bitsizetype, 8));
> +      rep_pos = fold_build2 (PLUS_EXPR, bitsizetype, rep_pos,
> +                            DECL_FIELD_BIT_OFFSET (rep_decl));
>
> you can use the invariant that DECL_FIELD_OFFSET of rep_decl
> and field_decl are always the same.  Also please use BITS_PER_UNIT
> instead of '8'.
>
> Richard.

[-- Attachment #2: PR107229-2.patch --]
[-- Type: text/plain, Size: 3681 bytes --]

diff --git a/gcc/testsuite/gcc.dg/vect/pr107229-1.c b/gcc/testsuite/gcc.dg/vect/pr107229-1.c
new file mode 100644
index 0000000000000000000000000000000000000000..67b432383d057a630746aa00af50c25fcb527d8e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/pr107229-1.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* PR tree-optimization/107229.  */
+
+int a, c;
+struct {
+  long d;
+  int : 8;
+  int : 27;
+  int e : 21;
+} f;
+void g(int b) { a = a & 1; }
+int main() {
+  while (c)
+    g(f.e);
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/vect/pr107229-2.c b/gcc/testsuite/gcc.dg/vect/pr107229-2.c
new file mode 100644
index 0000000000000000000000000000000000000000..88bffb63d5e8b2d7bcdeae223f4ec6ea4f611bc9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/pr107229-2.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* PR tree-optimization/107229.  */
+
+int a, c;
+struct {
+  long f;
+  long g;
+  long d;
+  int : 8;
+  int : 27;
+  int e : 21;
+} f;
+void g(int b) { a = a & 1; }
+int main() {
+  while (c)
+    g(f.e);
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/vect/pr107229-3.c b/gcc/testsuite/gcc.dg/vect/pr107229-3.c
new file mode 100644
index 0000000000000000000000000000000000000000..4abd8c14531b40e9dbe9802a8f9a0eabba673c9f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/pr107229-3.c
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* PR tree-optimization/107229.  */
+
+int a, c;
+struct {
+  long f;
+  long g;
+  long d;
+  int : 8;
+  int : 32;
+  int : 2;
+  int e : 21;
+} f;
+void g(int b) { a = a & 1; }
+int main() {
+  while (c)
+    g(f.e);
+  return 0;
+}
diff --git a/gcc/tree-if-conv.cc b/gcc/tree-if-conv.cc
index e468a4659fa28a3a31c3390cf19bee65f4590b80..01637c5da08d5a2a00a495522fc9a6436a804398 100644
--- a/gcc/tree-if-conv.cc
+++ b/gcc/tree-if-conv.cc
@@ -3298,10 +3298,34 @@ get_bitfield_rep (gassign *stmt, bool write, tree *bitpos,
     *struct_expr = TREE_OPERAND (comp_ref, 0);
 
   if (bitpos)
-    *bitpos
-      = fold_build2 (MINUS_EXPR, bitsizetype,
-		     DECL_FIELD_BIT_OFFSET (field_decl),
-		     DECL_FIELD_BIT_OFFSET (rep_decl));
+    {
+      /* To calculate the bitposition of the BITFIELD_REF we have to determine
+	 where our bitfield starts in relation to the container REP_DECL. The
+	 DECL_FIELD_OFFSET of the original bitfield's member FIELD_DECL tells
+	 us how many bytes from the start of the structure there are until the
+	 start of the group of bitfield members the FIELD_DECL belongs to,
+	 whereas DECL_FIELD_BIT_OFFSET will tell us how many bits from that
+	 position our actual bitfield member starts.  For the container
+	 REP_DECL adding DECL_FIELD_OFFSET and DECL_FIELD_BIT_OFFSET will tell
+	 us the distance between the start of the structure and the start of
+	 the container, though the first is in bytes and the later other in
+	 bits.  With this in mind we calculate the bit position of our new
+	 BITFIELD_REF by subtracting the number of bits between the start of
+	 the structure and the container from the number of bits from the start
+	 of the structure and the actual bitfield member. */
+      tree bf_pos = fold_build2 (MULT_EXPR, bitsizetype,
+				 DECL_FIELD_OFFSET (field_decl),
+				 build_int_cst (bitsizetype, BITS_PER_UNIT));
+      bf_pos = fold_build2 (PLUS_EXPR, bitsizetype, bf_pos,
+			    DECL_FIELD_BIT_OFFSET (field_decl));
+      tree rep_pos = fold_build2 (MULT_EXPR, bitsizetype,
+				  DECL_FIELD_OFFSET (rep_decl),
+				  build_int_cst (bitsizetype, BITS_PER_UNIT));
+      rep_pos = fold_build2 (PLUS_EXPR, bitsizetype, rep_pos,
+			     DECL_FIELD_BIT_OFFSET (rep_decl));
+
+      *bitpos = fold_build2 (MINUS_EXPR, bitsizetype, bf_pos, rep_pos);
+    }
 
   return rep_decl;