From patchwork Thu Jun 1 11:39:23 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 769642 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3wdlkQ4WGhz9s78 for ; Thu, 1 Jun 2017 21:39:42 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="Y8lFZRfL"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:mime-version:content-type; q=dns; s=default; b=UvYD6xAcPblfOtsvsjKb141Aj6zJzpTPEmqq2mrq4IGCUSbdl3 xo8pLtV66GJUtiqXpjaHjje/1QqWY2dv+CSbI0ILgc9BZ7SWvlxvlPIuzs2Vo752 VOwjgGzdySHbJE7BFZy4HC96TKiXUizix1eETD3McfxSLD8a0p9zMNyO8= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:mime-version:content-type; s= default; bh=sOMHwfOBgU2eJFlo09LsSvO8iqQ=; b=Y8lFZRfLXDiOZ8Ey82xF fyYyJY/r5NqoSMkYWV42ASwWLjhhnzUy6h85i/BYcUW3H3u9OThBmKN1C4a3Zed+ +Mv+8UvURy+wkTAW0gKpefNyxscGXVDVEWl4yGV5NIv4zZhNKP80FoUgogeleiY5 3wz3sbDw2y2Zkf7vaVzXHU0= Received: (qmail 36359 invoked by alias); 1 Jun 2017 11:39:26 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 36320 invoked by uid 89); 1 Jun 2017 11:39:24 -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, SPF_PASS autolearn=ham version=3.3.2 spammy=80000, upwards, REASON, sk:disqual 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:39:22 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 89375AD73 for ; Thu, 1 Jun 2017 11:39:24 +0000 (UTC) Date: Thu, 1 Jun 2017 13:39:23 +0200 From: Martin Jambor To: GCC Patches Cc: Richard Biener Subject: [PR 80898] Propagate grp_write from disqualified SRA candidates Message-ID: <20170601113923.rx3sdkm4xjik5zzt@virgil.suse.cz> Mail-Followup-To: GCC Patches , Richard Biener MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.6.2 (2016-07-01) X-IsSubscribed: yes 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? 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)