[PATCH] ifcvt: Don't lower bitfields with non-constant offsets [PR 111882]

[PATCH] ifcvt: Don't lower bitfields with non-constant offsets [PR 111882]

[Andre Vieira (lists)]

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

Hi,

This patch stops lowering of bitfields by ifcvt when they have non-constant
offsets as we are not likely to be able to do anything useful with those 
during
vectorization.  That also fixes the issue reported in PR 111882, which was
being caused by an offset with a side-effect being lowered, but 
constants have
no side-effects so we will no longer run into that problem.

Bootstrapped and regression tested on aarch64-unknown-linux-gnu.

OK for trunk?

gcc/ChangeLog:

	PR tree-optimization/111882
	* tree-if-conv.cc (get_bitfield_rep): Return NULL_TREE for bitfields with
	non-constant offsets.

gcc/testsuite/ChangeLog:

	* gcc.dg/vect/pr111882.c: New test.

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

diff --git a/gcc/testsuite/gcc.dg/vect/pr111882.c b/gcc/testsuite/gcc.dg/vect/pr111882.c
new file mode 100644
index 0000000000000000000000000000000000000000..024ad57b6930dd1f516e9c8127e2b440016360c6
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/pr111882.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-additional-options { -fdump-tree-ifcvt-all } } */
+
+static void __attribute__((noipa)) f(int n) {
+  int i, j;
+  struct S { char d[n]; int a; int b : 17; int c : 12; };
+  struct S A[100][1111];
+  for (i = 0; i < 100; i++) {
+    asm volatile("" : : "g"(&A[0][0]) : "memory");
+    for (j = 0; j < 1111; j++) A[i][j].b = 2;
+  }
+}
+void g(void) { f(1); }
+
+/* { dg-final { scan-tree-dump-not "Bitfield OK to lower" "ifcvt" } } */
diff --git a/gcc/tree-if-conv.cc b/gcc/tree-if-conv.cc
index dab7eeb7707ae8f1f342a571f8e5c99e0ef39309..66f3c882cb688049224b344b81637e7b1ae7e36c 100644
--- a/gcc/tree-if-conv.cc
+++ b/gcc/tree-if-conv.cc
@@ -3495,6 +3495,7 @@ get_bitfield_rep (gassign *stmt, bool write, tree *bitpos,
 			: gimple_assign_rhs1 (stmt);
 
   tree field_decl = TREE_OPERAND (comp_ref, 1);
+  tree ref_offset = component_ref_field_offset (comp_ref);
   tree rep_decl = DECL_BIT_FIELD_REPRESENTATIVE (field_decl);
 
   /* Bail out if the representative is not a suitable type for a scalar
@@ -3509,6 +3510,12 @@ get_bitfield_rep (gassign *stmt, bool write, tree *bitpos,
   if (compare_tree_int (DECL_SIZE (field_decl), bf_prec) != 0)
     return NULL_TREE;
 
+  if (!TREE_CONSTANT (DECL_FIELD_OFFSET (rep_decl))
+      || !TREE_CONSTANT (DECL_FIELD_BIT_OFFSET (rep_decl))
+      || !TREE_CONSTANT (ref_offset)
+      || !TREE_CONSTANT (DECL_FIELD_BIT_OFFSET (field_decl)))
+    return NULL_TREE;
+
   if (struct_expr)
     *struct_expr = TREE_OPERAND (comp_ref, 0);
 
@@ -3529,7 +3536,7 @@ get_bitfield_rep (gassign *stmt, bool write, tree *bitpos,
 	 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),
+				 ref_offset,
 				 build_int_cst (bitsizetype, BITS_PER_UNIT));
       bf_pos = fold_build2 (PLUS_EXPR, bitsizetype, bf_pos,
 			    DECL_FIELD_BIT_OFFSET (field_decl));

Re: [PATCH] ifcvt: Don't lower bitfields with non-constant offsets [PR 111882]

[Richard Biener]

On Fri, 20 Oct 2023, Andre Vieira (lists) wrote:

> Hi,
> 
> This patch stops lowering of bitfields by ifcvt when they have non-constant
> offsets as we are not likely to be able to do anything useful with those
> during
> vectorization.  That also fixes the issue reported in PR 111882, which was
> being caused by an offset with a side-effect being lowered, but constants have
> no side-effects so we will no longer run into that problem.
> 
> Bootstrapped and regression tested on aarch64-unknown-linux-gnu.
> 
> OK for trunk?

+  if (!TREE_CONSTANT (DECL_FIELD_OFFSET (rep_decl))
+      || !TREE_CONSTANT (DECL_FIELD_BIT_OFFSET (rep_decl))
+      || !TREE_CONSTANT (ref_offset)
+      || !TREE_CONSTANT (DECL_FIELD_BIT_OFFSET (field_decl)))
+    return NULL_TREE;

DECL_FIELD_BIT_OFFSET is always constant.  Please test
TREE_CODE (..) == INTEGER_CST instead of TREE_CONSTANT.


OK with those changes.

Richard.


> gcc/ChangeLog:
> 
> 	PR tree-optimization/111882
> 	* tree-if-conv.cc (get_bitfield_rep): Return NULL_TREE for bitfields
> 	with
> 	non-constant offsets.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/vect/pr111882.c: New test.
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

Re: [PATCH] ifcvt: Don't lower bitfields with non-constant offsets [PR 111882]

[Andre Vieira (lists)]

On 20/10/2023 14:41, Richard Biener wrote:
> On Fri, 20 Oct 2023, Andre Vieira (lists) wrote:
> 
>> Hi,
>>
>> This patch stops lowering of bitfields by ifcvt when they have non-constant
>> offsets as we are not likely to be able to do anything useful with those
>> during
>> vectorization.  That also fixes the issue reported in PR 111882, which was
>> being caused by an offset with a side-effect being lowered, but constants have
>> no side-effects so we will no longer run into that problem.
>>
>> Bootstrapped and regression tested on aarch64-unknown-linux-gnu.
>>
>> OK for trunk?
> 
> +  if (!TREE_CONSTANT (DECL_FIELD_OFFSET (rep_decl))
> +      || !TREE_CONSTANT (DECL_FIELD_BIT_OFFSET (rep_decl))
> +      || !TREE_CONSTANT (ref_offset)
> +      || !TREE_CONSTANT (DECL_FIELD_BIT_OFFSET (field_decl)))
> +    return NULL_TREE;
> 
> DECL_FIELD_BIT_OFFSET is always constant.  Please test
> TREE_CODE (..) == INTEGER_CST instead of TREE_CONSTANT.
> 
> 
> OK with those changes.
After I sent it I realized it would've been nicer to add a diagnostic, 
you OK with:
+      if (dump_file && (dump_flags & TDF_DETAILS))
+       fprintf (dump_file, "\t Bitfield NOT OK to lower,"
+                           " offset is non-constant.\n");
> 
> Richard.
> 
> 
>> gcc/ChangeLog:
>>
>> 	PR tree-optimization/111882
>> 	* tree-if-conv.cc (get_bitfield_rep): Return NULL_TREE for bitfields
>> 	with
>> 	non-constant offsets.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	* gcc.dg/vect/pr111882.c: New test.
>>
> 

Re: [PATCH] ifcvt: Don't lower bitfields with non-constant offsets [PR 111882]

[Richard Biener]

> Am 20.10.2023 um 15:47 schrieb Andre Vieira (lists) <andre.simoesdiasvieira@arm.com>:
> 
> 
> 
>> On 20/10/2023 14:41, Richard Biener wrote:
>>> On Fri, 20 Oct 2023, Andre Vieira (lists) wrote:
>>> Hi,
>>> 
>>> This patch stops lowering of bitfields by ifcvt when they have non-constant
>>> offsets as we are not likely to be able to do anything useful with those
>>> during
>>> vectorization.  That also fixes the issue reported in PR 111882, which was
>>> being caused by an offset with a side-effect being lowered, but constants have
>>> no side-effects so we will no longer run into that problem.
>>> 
>>> Bootstrapped and regression tested on aarch64-unknown-linux-gnu.
>>> 
>>> OK for trunk?
>> +  if (!TREE_CONSTANT (DECL_FIELD_OFFSET (rep_decl))
>> +      || !TREE_CONSTANT (DECL_FIELD_BIT_OFFSET (rep_decl))
>> +      || !TREE_CONSTANT (ref_offset)
>> +      || !TREE_CONSTANT (DECL_FIELD_BIT_OFFSET (field_decl)))
>> +    return NULL_TREE;
>> DECL_FIELD_BIT_OFFSET is always constant.  Please test
>> TREE_CODE (..) == INTEGER_CST instead of TREE_CONSTANT.
>> OK with those changes.
> After I sent it I realized it would've been nicer to add a diagnostic, you OK with:
> +      if (dump_file && (dump_flags & TDF_DETAILS))
> +       fprintf (dump_file, "\t Bitfield NOT OK to lower,"
> +                           " offset is non-constant.\n");

Sure.

>> Richard.
>>> gcc/ChangeLog:
>>> 
>>>    PR tree-optimization/111882
>>>    * tree-if-conv.cc (get_bitfield_rep): Return NULL_TREE for bitfields
>>>    with
>>>    non-constant offsets.
>>> 
>>> gcc/testsuite/ChangeLog:
>>> 
>>>    * gcc.dg/vect/pr111882.c: New test.
>>>