Re: [PATCH] tree-optimization/111779 - Handle some BIT_FIELD_REFs in SRA
Hi Richard, being the one who wrote the surrounding code: The fortran part looks good to me. Ok for merge from the fortran side. - Andre On Thu, 12 Oct 2023 11:44:01 + (UTC) Richard Biener wrote: > On Thu, 12 Oct 2023, Richard Biener wrote: > > > The following handles byte-aligned, power-of-two and byte-multiple > > sized BIT_FIELD_REF reads in SRA. In particular this should cover > > BIT_FIELD_REFs created by optimize_bit_field_compare. > > > > For gcc.dg/tree-ssa/ssa-dse-26.c we now SRA the BIT_FIELD_REF > > appearing there leading to more DSE, fully eliding the aggregates. > > > > This results in the same false positive -Wuninitialized as the > > older attempt to remove the folding from optimize_bit_field_compare, > > fixed by initializing part of the aggregate unconditionally. > > > > Bootstrapped and tested on x86_64-unknown-linux-gnu for all languages. > > > > Martin is on leave so I'll push this tomorrow unless the Fortran > > folks have objections. > > Err, and I forgot that hunk. It's > > diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc > index 7beefa2e69c..1b8be081a17 100644 > --- a/gcc/fortran/trans-expr.cc > +++ b/gcc/fortran/trans-expr.cc > @@ -12015,7 +12015,10 @@ gfc_trans_assignment_1 (gfc_expr * expr1, gfc_expr * > expr2, bool init_flag, && !is_runtime_conformable (expr1, expr2); > >/* Only analyze the expressions for coarray properties, when in coarray-lib > - mode. */ > + mode. Avoid false-positive uninitialized diagnostics with initializing > + the codimension flag unconditionally. */ > + lhs_caf_attr.codimension = false; > + rhs_caf_attr.codimension = false; >if (flag_coarray == GFC_FCOARRAY_LIB) > { >lhs_caf_attr = gfc_caf_attr (expr1, false, &lhs_refs_comp); > > > > Thanks, > > Richard. > > > > PR tree-optimization/111779 > > gcc/ > > * tree-sra.cc (sra_handled_bf_read_p): New function. > > (build_access_from_expr_1): Handle some BIT_FIELD_REFs. > > (sra_modify_expr): Likewise. > > (make_fancy_name_1): Skip over BIT_FIELD_REF. > > > > gcc/fortran/ > > * trans-expr.cc (gfc_trans_assignment_1): Initialize > > lhs_caf_attr and rhs_caf_attr codimension flag to avoid > > false positive -Wuninitialized. > > > > gcc/testsuite/ > > * gcc.dg/tree-ssa/ssa-dse-26.c: Adjust for more DSE. > > * gcc.dg/vect/vect-pr111779.c: New testcase. > > --- > > gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-26.c | 4 +- > > gcc/testsuite/gcc.dg/vect/vect-pr111779.c | 56 ++ > > gcc/tree-sra.cc| 24 -- > > 3 files changed, 79 insertions(+), 5 deletions(-) > > create mode 100644 gcc/testsuite/gcc.dg/vect/vect-pr111779.c > > > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-26.c > > b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-26.c index e3c33f49ef6..43152de5616 > > 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-26.c > > +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-26.c > > @@ -31,5 +31,5 @@ constraint_equal (struct constraint a, struct constraint > > b) && constraint_expr_equal (a.rhs, b.rhs); > > } > > > > -/* { dg-final { scan-tree-dump-times "Deleted dead store: x = " 1 "dse1" } > > } */ -/* { dg-final { scan-tree-dump-times "Deleted dead store: y = " 1 > > "dse1" } } */ +/* { dg-final { scan-tree-dump-times "Deleted dead store: x > > = " 2 "dse1" } } */ +/* { dg-final { scan-tree-dump-times "Deleted dead > > store: y = " 2 "dse1" } } */ diff --git > > a/gcc/testsuite/gcc.dg/vect/vect-pr111779.c > > b/gcc/testsuite/gcc.dg/vect/vect-pr111779.c new file mode 100644 index > > 000..79b72aebc78 --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/vect/vect-pr111779.c > > @@ -0,0 +1,56 @@ > > +#include > > +#include "tree-vect.h" > > + > > +struct C > > +{ > > +int c; > > +int d; > > +bool f :1; > > +float e; > > +}; > > + > > +struct A > > +{ > > + unsigned int a; > > + unsigned char c1, c2; > > + bool b1 : 1; > > + bool b2 : 1; > > + bool b3 : 1; > > + struct C b4; > > +}; > > + > > +void __attribute__((noipa)) > > +foo (const struct A * __restrict x, int y) > > +{ > > + int s = 0, i = 0; > > + for (i = 0; i < y; ++i) > > +{ > > + const struct A a = x[i]; > > + s += a.b4.f ? 1 : 0; > > +} > > + if (s != 0) > > +__builtin_abort (); > > +} > > + > > +int > > +main () > > +{ > > + struct A x[100]; > > + int i; > > + > > + check_vect (); > > + > > + __builtin_memset (x, -1, sizeof (x)); > > +#pragma GCC novect > > + for (i = 0; i < 100; i++) > > +{ > > + x[i].b1 = false; > > + x[i].b2 = false; > > + x[i].b3 = false; > > + x[i].b4.f = false; > > +} > > + foo (x, 100); > > + return 0; > > +} > > + > > +/* { dg-final { scan-tree-dump "LOOP VECTORIZED" "vect" { target vect_int > > } } } */ diff --git a/gcc/tree-sra.cc b/gcc/tree-sra.cc > > index 56a8ba26135..24d0c20da6a 100644 > > --- a/gcc/tree-sra.cc > > +++ b/gcc/tree-sra.cc > > @@ -1113,6 +1113,21 @@ disqualify_base_of_expr
Re: [PATCH] tree-optimization/111779 - Handle some BIT_FIELD_REFs in SRA
On Thu, 12 Oct 2023, Richard Biener wrote: > The following handles byte-aligned, power-of-two and byte-multiple > sized BIT_FIELD_REF reads in SRA. In particular this should cover > BIT_FIELD_REFs created by optimize_bit_field_compare. > > For gcc.dg/tree-ssa/ssa-dse-26.c we now SRA the BIT_FIELD_REF > appearing there leading to more DSE, fully eliding the aggregates. > > This results in the same false positive -Wuninitialized as the > older attempt to remove the folding from optimize_bit_field_compare, > fixed by initializing part of the aggregate unconditionally. > > Bootstrapped and tested on x86_64-unknown-linux-gnu for all languages. > > Martin is on leave so I'll push this tomorrow unless the Fortran > folks have objections. Err, and I forgot that hunk. It's diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 7beefa2e69c..1b8be081a17 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -12015,7 +12015,10 @@ gfc_trans_assignment_1 (gfc_expr * expr1, gfc_expr * expr2, bool init_flag, && !is_runtime_conformable (expr1, expr2); /* Only analyze the expressions for coarray properties, when in coarray-lib - mode. */ + mode. Avoid false-positive uninitialized diagnostics with initializing + the codimension flag unconditionally. */ + lhs_caf_attr.codimension = false; + rhs_caf_attr.codimension = false; if (flag_coarray == GFC_FCOARRAY_LIB) { lhs_caf_attr = gfc_caf_attr (expr1, false, &lhs_refs_comp); > Thanks, > Richard. > > PR tree-optimization/111779 > gcc/ > * tree-sra.cc (sra_handled_bf_read_p): New function. > (build_access_from_expr_1): Handle some BIT_FIELD_REFs. > (sra_modify_expr): Likewise. > (make_fancy_name_1): Skip over BIT_FIELD_REF. > > gcc/fortran/ > * trans-expr.cc (gfc_trans_assignment_1): Initialize > lhs_caf_attr and rhs_caf_attr codimension flag to avoid > false positive -Wuninitialized. > > gcc/testsuite/ > * gcc.dg/tree-ssa/ssa-dse-26.c: Adjust for more DSE. > * gcc.dg/vect/vect-pr111779.c: New testcase. > --- > gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-26.c | 4 +- > gcc/testsuite/gcc.dg/vect/vect-pr111779.c | 56 ++ > gcc/tree-sra.cc| 24 -- > 3 files changed, 79 insertions(+), 5 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/vect/vect-pr111779.c > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-26.c > b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-26.c > index e3c33f49ef6..43152de5616 100644 > --- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-26.c > +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-26.c > @@ -31,5 +31,5 @@ constraint_equal (struct constraint a, struct constraint b) > && constraint_expr_equal (a.rhs, b.rhs); > } > > -/* { dg-final { scan-tree-dump-times "Deleted dead store: x = " 1 "dse1" } } > */ > -/* { dg-final { scan-tree-dump-times "Deleted dead store: y = " 1 "dse1" } } > */ > +/* { dg-final { scan-tree-dump-times "Deleted dead store: x = " 2 "dse1" } } > */ > +/* { dg-final { scan-tree-dump-times "Deleted dead store: y = " 2 "dse1" } } > */ > diff --git a/gcc/testsuite/gcc.dg/vect/vect-pr111779.c > b/gcc/testsuite/gcc.dg/vect/vect-pr111779.c > new file mode 100644 > index 000..79b72aebc78 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/vect/vect-pr111779.c > @@ -0,0 +1,56 @@ > +#include > +#include "tree-vect.h" > + > +struct C > +{ > +int c; > +int d; > +bool f :1; > +float e; > +}; > + > +struct A > +{ > + unsigned int a; > + unsigned char c1, c2; > + bool b1 : 1; > + bool b2 : 1; > + bool b3 : 1; > + struct C b4; > +}; > + > +void __attribute__((noipa)) > +foo (const struct A * __restrict x, int y) > +{ > + int s = 0, i = 0; > + for (i = 0; i < y; ++i) > +{ > + const struct A a = x[i]; > + s += a.b4.f ? 1 : 0; > +} > + if (s != 0) > +__builtin_abort (); > +} > + > +int > +main () > +{ > + struct A x[100]; > + int i; > + > + check_vect (); > + > + __builtin_memset (x, -1, sizeof (x)); > +#pragma GCC novect > + for (i = 0; i < 100; i++) > +{ > + x[i].b1 = false; > + x[i].b2 = false; > + x[i].b3 = false; > + x[i].b4.f = false; > +} > + foo (x, 100); > + return 0; > +} > + > +/* { dg-final { scan-tree-dump "LOOP VECTORIZED" "vect" { target vect_int } > } } */ > diff --git a/gcc/tree-sra.cc b/gcc/tree-sra.cc > index 56a8ba26135..24d0c20da6a 100644 > --- a/gcc/tree-sra.cc > +++ b/gcc/tree-sra.cc > @@ -1113,6 +1113,21 @@ disqualify_base_of_expr (tree t, const char *reason) > disqualify_candidate (t, reason); > } > > +/* Return true if the BIT_FIELD_REF read EXPR is handled by SRA. */ > + > +static bool > +sra_handled_bf_read_p (tree expr) > +{ > + uint64_t size, offset; > + if (bit_field_size (expr).is_constant (&size) > + && bit_field_offset (expr).is_constant (&offset) > + && size % BITS_PER_UNIT == 0 > + && offset % B
[PATCH] tree-optimization/111779 - Handle some BIT_FIELD_REFs in SRA
The following handles byte-aligned, power-of-two and byte-multiple sized BIT_FIELD_REF reads in SRA. In particular this should cover BIT_FIELD_REFs created by optimize_bit_field_compare. For gcc.dg/tree-ssa/ssa-dse-26.c we now SRA the BIT_FIELD_REF appearing there leading to more DSE, fully eliding the aggregates. This results in the same false positive -Wuninitialized as the older attempt to remove the folding from optimize_bit_field_compare, fixed by initializing part of the aggregate unconditionally. Bootstrapped and tested on x86_64-unknown-linux-gnu for all languages. Martin is on leave so I'll push this tomorrow unless the Fortran folks have objections. Thanks, Richard. PR tree-optimization/111779 gcc/ * tree-sra.cc (sra_handled_bf_read_p): New function. (build_access_from_expr_1): Handle some BIT_FIELD_REFs. (sra_modify_expr): Likewise. (make_fancy_name_1): Skip over BIT_FIELD_REF. gcc/fortran/ * trans-expr.cc (gfc_trans_assignment_1): Initialize lhs_caf_attr and rhs_caf_attr codimension flag to avoid false positive -Wuninitialized. gcc/testsuite/ * gcc.dg/tree-ssa/ssa-dse-26.c: Adjust for more DSE. * gcc.dg/vect/vect-pr111779.c: New testcase. --- gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-26.c | 4 +- gcc/testsuite/gcc.dg/vect/vect-pr111779.c | 56 ++ gcc/tree-sra.cc| 24 -- 3 files changed, 79 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/vect/vect-pr111779.c diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-26.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-26.c index e3c33f49ef6..43152de5616 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-26.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-26.c @@ -31,5 +31,5 @@ constraint_equal (struct constraint a, struct constraint b) && constraint_expr_equal (a.rhs, b.rhs); } -/* { dg-final { scan-tree-dump-times "Deleted dead store: x = " 1 "dse1" } } */ -/* { dg-final { scan-tree-dump-times "Deleted dead store: y = " 1 "dse1" } } */ +/* { dg-final { scan-tree-dump-times "Deleted dead store: x = " 2 "dse1" } } */ +/* { dg-final { scan-tree-dump-times "Deleted dead store: y = " 2 "dse1" } } */ diff --git a/gcc/testsuite/gcc.dg/vect/vect-pr111779.c b/gcc/testsuite/gcc.dg/vect/vect-pr111779.c new file mode 100644 index 000..79b72aebc78 --- /dev/null +++ b/gcc/testsuite/gcc.dg/vect/vect-pr111779.c @@ -0,0 +1,56 @@ +#include +#include "tree-vect.h" + +struct C +{ +int c; +int d; +bool f :1; +float e; +}; + +struct A +{ + unsigned int a; + unsigned char c1, c2; + bool b1 : 1; + bool b2 : 1; + bool b3 : 1; + struct C b4; +}; + +void __attribute__((noipa)) +foo (const struct A * __restrict x, int y) +{ + int s = 0, i = 0; + for (i = 0; i < y; ++i) +{ + const struct A a = x[i]; + s += a.b4.f ? 1 : 0; +} + if (s != 0) +__builtin_abort (); +} + +int +main () +{ + struct A x[100]; + int i; + + check_vect (); + + __builtin_memset (x, -1, sizeof (x)); +#pragma GCC novect + for (i = 0; i < 100; i++) +{ + x[i].b1 = false; + x[i].b2 = false; + x[i].b3 = false; + x[i].b4.f = false; +} + foo (x, 100); + return 0; +} + +/* { dg-final { scan-tree-dump "LOOP VECTORIZED" "vect" { target vect_int } } } */ diff --git a/gcc/tree-sra.cc b/gcc/tree-sra.cc index 56a8ba26135..24d0c20da6a 100644 --- a/gcc/tree-sra.cc +++ b/gcc/tree-sra.cc @@ -1113,6 +1113,21 @@ disqualify_base_of_expr (tree t, const char *reason) disqualify_candidate (t, reason); } +/* Return true if the BIT_FIELD_REF read EXPR is handled by SRA. */ + +static bool +sra_handled_bf_read_p (tree expr) +{ + uint64_t size, offset; + if (bit_field_size (expr).is_constant (&size) + && bit_field_offset (expr).is_constant (&offset) + && size % BITS_PER_UNIT == 0 + && offset % BITS_PER_UNIT == 0 + && pow2p_hwi (size)) +return true; + return false; +} + /* Scan expression EXPR and create access structures for all accesses to candidates for scalarization. Return the created access or NULL if none is created. */ @@ -1123,7 +1138,8 @@ build_access_from_expr_1 (tree expr, gimple *stmt, bool write) struct access *ret = NULL; bool partial_ref; - if (TREE_CODE (expr) == BIT_FIELD_REF + if ((TREE_CODE (expr) == BIT_FIELD_REF + && (write || !sra_handled_bf_read_p (expr))) || TREE_CODE (expr) == IMAGPART_EXPR || TREE_CODE (expr) == REALPART_EXPR) { @@ -1170,6 +1186,7 @@ build_access_from_expr_1 (tree expr, gimple *stmt, bool write) case COMPONENT_REF: case ARRAY_REF: case ARRAY_RANGE_REF: +case BIT_FIELD_REF: ret = create_access (expr, stmt, write); break; @@ -1549,6 +1566,7 @@ make_fancy_name_1 (tree expr) obstack_grow (&name_obstack, buffer, strlen (buffer)); break; +case BIT_FIELD_REF: case ADDR_EXPR: make_fancy_name_1 (TREE_OPER