| Message ID | 29e81713-e0a7-47da-bb4b-01b6629962c1@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | ifcvt: Don't lower bitfields with non-constant offsets [PR 111882] | expand |
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. >
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. >> >
> 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. >>>
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));