From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x530.google.com (mail-ed1-x530.google.com [IPv6:2a00:1450:4864:20::530]) by sourceware.org (Postfix) with ESMTPS id 686A83858D39 for ; Thu, 21 Oct 2021 07:40:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 686A83858D39 Received: by mail-ed1-x530.google.com with SMTP id z20so278825edc.13 for ; Thu, 21 Oct 2021 00:40:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=bJdNgtsJK1YcyJWdzbVxL2edZ+gqY1HdiAQKctb2YPk=; b=OL7Z45q4q4ksRqJlWXN+NQNgJbdtMDaqhrn9Xl1YsdUqEA9leIhgm5m1kmloaxIu0o RmCgh27rVG3PYj1bTmvwBq4xDDrvNA/eTmzevBJM9DQq9O7Tv4HT/GQS/xoEt7T4gzRs 066dHHdjSfxvTCMMPUqvCGHJcafU55047X5mdQ5zGyj8pLu+alwaeZEv2LJFjakrxEuK Ku7KtkLcu+Gd1vTSoHZyqKRgGZOvcczGQcyeBZPBtj8j5hfZBTAqvgKu3P5+mYovG6Pn /snPE1+NwEI9hnE/78I5B5dKk0ZeTGoZPN0uDIZGgr9MafymgSV6ixm3UCMzNqXDi0kD tNgA== X-Gm-Message-State: AOAM532oZox1oPgfiMaLMLSRnT3kTeYQ7YEoIEGZ8jXpWJO8NQv7KAwt ZtCGU1BnD3FYOjBif+BQUix5gNZCxEXZTmS1aGm5HOc8 X-Google-Smtp-Source: ABdhPJzm9aJNngztzK/56p9eKbfbBh+ywQaH9FrSXlTxNsqmX+0k5cJZEX/aiOWew7PlTLO3nLA5596/tUQyMif+AK0= X-Received: by 2002:a05:6402:27d3:: with SMTP id c19mr5584873ede.70.1634802052370; Thu, 21 Oct 2021 00:40:52 -0700 (PDT) MIME-Version: 1.0 References: <1634795547-24044-1-git-send-email-apinski@marvell.com> In-Reply-To: <1634795547-24044-1-git-send-email-apinski@marvell.com> From: Richard Biener Date: Thu, 21 Oct 2021 09:40:41 +0200 Message-ID: Subject: Re: [PATCH] Improve maybe_remove_writeonly_store to do a simple DCE for defining statement To: Andrew Pinski Cc: GCC Patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-8.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 21 Oct 2021 07:40:55 -0000 On Thu, Oct 21, 2021 at 7:53 AM apinski--- via Gcc-patches wrote: > > From: Andrew Pinski > > Instead of putting a full blow DCE after execute_fixup_cfg, it makes sense > to try to remove the defining statement for the store that is being removed. > Using simple_dce_from_worklist makes this easier, just mark the ssa_name on > the rhs side of the store (if it was one) in a bitmap and then call > simple_dce_from_worklist at the end. > > gcc.dg/pr36902.c needed to be changed such that the static array was no > longer a static array but a global array. This is because this new dce > will remove the load as it is dead. I also filed PR 102864 for the warning > on dead loads. OK. Thanks, Richard. > gcc/ChangeLog: > > * tree-cfg.c (maybe_remove_writeonly_store): Add dce_ssa_names argument. > Mark the ssa-name of the rhs as one to be removed. > (execute_fixup_cfg): Update call to maybe_remove_writeonly_store. > Call simple_dce_from_worklist at the end to a simple dce. > > gcc/testsuite/ChangeLog: > > * gcc.dg/pr36902.c: Move buf to be a non-static variable. > --- > gcc/testsuite/gcc.dg/pr36902.c | 5 +---- > gcc/tree-cfg.c | 18 ++++++++++++++++-- > 2 files changed, 17 insertions(+), 6 deletions(-) > > diff --git a/gcc/testsuite/gcc.dg/pr36902.c b/gcc/testsuite/gcc.dg/pr36902.c > index 7dafc9ac171..365a26e26b7 100644 > --- a/gcc/testsuite/gcc.dg/pr36902.c > +++ b/gcc/testsuite/gcc.dg/pr36902.c > @@ -24,10 +24,9 @@ struct { > unsigned char pcr_select[4]; > } sel; > > +unsigned char buf[64]; > int bar(void) > { > - static unsigned char buf[64]; > - > sel.size_of_select = 3; > foo(buf, sel.pcr_select, sel.size_of_select); > > @@ -52,8 +51,6 @@ foo2(unsigned char * to, const unsigned char * from, int n) > > int baz(void) > { > - static unsigned char buf[64]; > - > sel.size_of_select = 5; > foo2(buf, sel.pcr_select, sel.size_of_select); > > diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c > index dbbf6beb6e4..b3a27bcd17c 100644 > --- a/gcc/tree-cfg.c > +++ b/gcc/tree-cfg.c > @@ -54,6 +54,7 @@ along with GCC; see the file COPYING3. If not see > #include "value-prof.h" > #include "tree-inline.h" > #include "tree-ssa-live.h" > +#include "tree-ssa-dce.h" > #include "omp-general.h" > #include "omp-expand.h" > #include "tree-cfgcleanup.h" > @@ -9669,7 +9670,8 @@ make_pass_warn_unused_result (gcc::context *ctxt) > /* Maybe Remove stores to variables we marked write-only. > Return true if a store was removed. */ > static bool > -maybe_remove_writeonly_store (gimple_stmt_iterator &gsi, gimple *stmt) > +maybe_remove_writeonly_store (gimple_stmt_iterator &gsi, gimple *stmt, > + bitmap dce_ssa_names) > { > /* Keep access when store has side effect, i.e. in case when source > is volatile. */ > @@ -9692,6 +9694,15 @@ maybe_remove_writeonly_store (gimple_stmt_iterator &gsi, gimple *stmt) > print_gimple_stmt (dump_file, stmt, 0, > TDF_VOPS|TDF_MEMSYMS); > } > + > + /* Mark ssa name defining to be checked for simple dce. */ > + if (gimple_assign_single_p (stmt)) > + { > + tree rhs = gimple_assign_rhs1 (stmt); > + if (TREE_CODE (rhs) == SSA_NAME > + && !SSA_NAME_IS_DEFAULT_DEF (rhs)) > + bitmap_set_bit (dce_ssa_names, SSA_NAME_VERSION (rhs)); > + } > unlink_stmt_vdef (stmt); > gsi_remove (&gsi, true); > release_defs (stmt); > @@ -9714,6 +9725,7 @@ execute_fixup_cfg (void) > profile_count num = node->count; > profile_count den = ENTRY_BLOCK_PTR_FOR_FN (cfun)->count; > bool scale = num.initialized_p () && !(num == den); > + auto_bitmap dce_ssa_names; > > if (scale) > { > @@ -9754,7 +9766,7 @@ execute_fixup_cfg (void) > } > > /* Remove stores to variables we marked write-only. */ > - if (maybe_remove_writeonly_store (gsi, stmt)) > + if (maybe_remove_writeonly_store (gsi, stmt, dce_ssa_names)) > { > todo |= TODO_update_ssa | TODO_cleanup_cfg; > continue; > @@ -9820,6 +9832,8 @@ execute_fixup_cfg (void) > && (todo & TODO_cleanup_cfg)) > loops_state_set (LOOPS_NEED_FIXUP); > > + simple_dce_from_worklist (dce_ssa_names); > + > return todo; > } > > -- > 2.17.1 >