From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 42013 invoked by alias); 1 Jun 2017 11:52:02 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 42002 invoked by uid 89); 1 Jun 2017 11:52:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 01 Jun 2017 11:51:59 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 23F4AAAB9 for ; Thu, 1 Jun 2017 11:52:01 +0000 (UTC) Date: Thu, 01 Jun 2017 11:52:00 -0000 From: Richard Biener To: Martin Jambor cc: GCC Patches Subject: Re: [PR 80898] Propagate grp_write from disqualified SRA candidates In-Reply-To: <20170601113923.rx3sdkm4xjik5zzt@virgil.suse.cz> Message-ID: References: <20170601113923.rx3sdkm4xjik5zzt@virgil.suse.cz> User-Agent: Alpine 2.20 (LSU 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-SW-Source: 2017-06/txt/msg00029.txt.bz2 On Thu, 1 Jun 2017, Martin Jambor wrote: > Hi, > > when I wrote the lazy setting of grp_write flag early next year, I > made a mistake when thinking about what to do about SRA candidates > that were disqualified but form a RHS of an assignment link which was > to be used to set grp_write of the LHS when appropriate. The code > expects that the RHS accesses form an access tree, but given that some > are rejected exactly because such a tree cannot be built, it does not > work. > > The solution is to move dealing with disqualified RHSs to the > assignment link processing. The patch below checks RHS and if it is > disqualified, marks the corresponding LHS as containing data. As the > second testcase shows, that information must be then also propagated > downwards (this is not necessary in the normal propagation case > because there propagate_subaccesses_across_link will already do that > more elaborately) as well as upwards. > > Bootstrapped and tested on x86_64-linux without any issues. OK for > trunk? Ok. Thanks, Richard. > > Thanks, > > Martin > > > 2017-06-01 Martin Jambor > > PR tree-optimization/80898 > * tree-sra.c (process_subtree_disqualification): Removed. > (disqualify_candidate): Do not call > process_subtree_disqualification. > (subtree_mark_written_and_enqueue): New function. > (propagate_all_subaccesses): Set grp_write of LHS subtree if the > RHS has been disqualified and re-queue LHS if necessary. Apart > from that, ignore disqualified RHS. > > testsuite/ > * gcc.dg/tree-ssa/pr80898.c: New test. > * gcc.dg/tree-ssa/pr80898-2.c: Likewise. > --- > gcc/testsuite/gcc.dg/tree-ssa/pr80898-2.c | 71 +++++++++++++++++++++++++++++++ > gcc/testsuite/gcc.dg/tree-ssa/pr80898.c | 20 +++++++++ > gcc/tree-sra.c | 56 +++++++++++++++--------- > 3 files changed, 126 insertions(+), 21 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr80898-2.c > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr80898.c > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr80898-2.c b/gcc/testsuite/gcc.dg/tree-ssa/pr80898-2.c > new file mode 100644 > index 00000000000..cb4799c5ced > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr80898-2.c > @@ -0,0 +1,71 @@ > +/* { dg-do run } */ > +/* { dg-options "-O2" } */ > + > +struct S0 > +{ > + unsigned a : 15; > + int b; > + int c; > +}; > + > +struct S1 > +{ > + struct S0 s0; > + int e; > +}; > + > +struct Z > +{ > + char c; > + int z; > +} __attribute__((packed)); > + > +union U > +{ > + struct S1 s1; > + struct Z z; > +}; > + > + > +int __attribute__((noinline, noclone)) > +return_zero (void) > +{ > + return 0; > +} > + > +volatile union U gu; > +struct S0 gs; > + > +int __attribute__((noinline, noclone)) > +check_outcome () > +{ > + if (gs.a != 6 > + || gs.b != 80000) > + __builtin_abort (); > +} > + > +int > +main (int argc, char *argv[]) > +{ > + union U u; > + struct S1 m; > + struct S0 l; > + > + if (return_zero ()) > + u.z.z = 20000; > + else > + { > + u.s1.s0.a = 6; > + u.s1.s0.b = 80000; > + u.s1.e = 2; > + > + m = u.s1; > + m.s0.c = 0; > + l = m.s0; > + gs = l; > + } > + > + gu = u; > + check_outcome (); > + return 0; > +} > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr80898.c b/gcc/testsuite/gcc.dg/tree-ssa/pr80898.c > new file mode 100644 > index 00000000000..ed88f2cbd1a > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr80898.c > @@ -0,0 +1,20 @@ > +/* { dg-do run } */ > +/* { dg-options "-O2" } */ > + > +struct S0 { > + int f0 : 24; > + int f1; > + int f74; > +} a, *c = &a; > +struct S0 fn1() { > + struct S0 b = {4, 3}; > + return b; > +} > + > +int main() { > + *c = fn1(); > + > + if (a.f1 != 3) > + __builtin_abort (); > + return 0; > +} > diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c > index 6a8a0a4a427..f25818f4481 100644 > --- a/gcc/tree-sra.c > +++ b/gcc/tree-sra.c > @@ -694,21 +694,9 @@ static bool constant_decl_p (tree decl) > return VAR_P (decl) && DECL_IN_CONSTANT_POOL (decl); > } > > - > -/* Mark LHS of assign links out of ACCESS and its children as written to. */ > - > -static void > -process_subtree_disqualification (struct access *access) > -{ > - struct access *child; > - for (struct assign_link *link = access->first_link; link; link = link->next) > - link->lacc->grp_write = true; > - for (child = access->first_child; child; child = child->next_sibling) > - process_subtree_disqualification (child); > -} > - > /* Remove DECL from candidates for SRA and write REASON to the dump file if > there is one. */ > + > static void > disqualify_candidate (tree decl, const char *reason) > { > @@ -723,13 +711,6 @@ disqualify_candidate (tree decl, const char *reason) > print_generic_expr (dump_file, decl); > fprintf (dump_file, " - %s\n", reason); > } > - > - struct access *access = get_first_repr_for_decl (decl); > - while (access) > - { > - process_subtree_disqualification (access); > - access = access->next_grp; > - } > } > > /* Return true iff the type contains a field or an element which does not allow > @@ -2679,6 +2660,26 @@ propagate_subaccesses_across_link (struct access *lacc, struct access *racc) > return ret; > } > > +/* Beginning with ACCESS, traverse its whole access subtree and mark all > + sub-trees as written to. If any of them has not been marked so previously > + and has assignment links leading from it, re-enqueue it. */ > + > +static void > +subtree_mark_written_and_enqueue (struct access *access) > +{ > + if (access->grp_write) > + return; > + access->grp_write = true; > + if (access->first_link) > + add_access_to_work_queue (access); > + > + struct access *child; > + for (child = access->first_child; child; child = child->next_sibling) > + subtree_mark_written_and_enqueue (child); > +} > + > + > + > /* Propagate all subaccesses across assignment links. */ > > static void > @@ -2698,7 +2699,20 @@ propagate_all_subaccesses (void) > if (!bitmap_bit_p (candidate_bitmap, DECL_UID (lacc->base))) > continue; > lacc = lacc->group_representative; > - if (propagate_subaccesses_across_link (lacc, racc)) > + > + bool reque_parents = false; > + if (!bitmap_bit_p (candidate_bitmap, DECL_UID (racc->base))) > + { > + if (!lacc->grp_write) > + { > + subtree_mark_written_and_enqueue (lacc); > + reque_parents = true; > + } > + } > + else if (propagate_subaccesses_across_link (lacc, racc)) > + reque_parents = true; > + > + if (reque_parents) > do > { > if (lacc->first_link) > -- Richard Biener SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)