From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id A1B163858C5E; Thu, 12 Oct 2023 11:44:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A1B163858C5E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id 31F971F74B; Thu, 12 Oct 2023 11:44:01 +0000 (UTC) Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 011082C993; Thu, 12 Oct 2023 11:44:01 +0000 (UTC) Date: Thu, 12 Oct 2023 11:44:01 +0000 (UTC) From: Richard Biener To: gcc-patches@gcc.gnu.org cc: Martin Jambor , fortran@gcc.gnu.org Subject: Re: [PATCH] tree-optimization/111779 - Handle some BIT_FIELD_REFs in SRA Message-ID: User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Level: Authentication-Results: smtp-out2.suse.de; dkim=none; dmarc=none; spf=softfail (smtp-out2.suse.de: 149.44.160.134 is neither permitted nor denied by domain of rguenther@suse.de) smtp.mailfrom=rguenther@suse.de X-Rspamd-Server: rspamd2 X-Spamd-Result: default: False [-1.51 / 50.00]; ARC_NA(0.00)[]; FAKE_REPLY(1.00)[]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[3]; TO_DN_SOME(0.00)[]; NEURAL_HAM_LONG(-3.00)[-1.000]; MIME_GOOD(-0.10)[text/plain]; DMARC_NA(0.20)[suse.de]; R_SPF_SOFTFAIL(0.60)[~all:c]; TO_MATCH_ENVRCPT_SOME(0.00)[]; VIOLATED_DIRECT_SPF(3.50)[]; MX_GOOD(-0.01)[]; NEURAL_HAM_SHORT(-1.00)[-1.000]; RWL_MAILSPIKE_GOOD(0.00)[149.44.160.134:from]; RCVD_NO_TLS_LAST(0.10)[]; FROM_EQ_ENVFROM(0.00)[]; R_DKIM_NA(0.20)[]; MIME_TRACE(0.00)[0:+]; RCVD_COUNT_TWO(0.00)[2]; BAYES_HAM(-3.00)[100.00%] X-Spam-Score: -1.51 X-Rspamd-Queue-Id: 31F971F74B X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_STATUS,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 00000000000..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_OPERAND (expr, 0)); > break; > @@ -1564,7 +1582,6 @@ make_fancy_name_1 (tree expr) > } > break; > > - case BIT_FIELD_REF: > case REALPART_EXPR: > case IMAGPART_EXPR: > gcc_unreachable (); /* we treat these as scalars. */ > @@ -3769,7 +3786,8 @@ sra_modify_expr (tree *expr, gimple_stmt_iterator *gsi, bool write) > tree type, bfr, orig_expr; > bool partial_cplx_access = false; > > - if (TREE_CODE (*expr) == BIT_FIELD_REF) > + if (TREE_CODE (*expr) == BIT_FIELD_REF > + && (write || !sra_handled_bf_read_p (*expr))) > { > bfr = *expr; > expr = &TREE_OPERAND (*expr, 0); > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)