From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x22e.google.com (mail-lj1-x22e.google.com [IPv6:2a00:1450:4864:20::22e]) by sourceware.org (Postfix) with ESMTPS id 6774A3857720 for ; Thu, 27 Apr 2023 10:52:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6774A3857720 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-lj1-x22e.google.com with SMTP id 38308e7fff4ca-2a7b02615f1so82644391fa.0 for ; Thu, 27 Apr 2023 03:52:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1682592722; x=1685184722; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=j7spBiryByRXoOgHwqaNuAV486OshQIV2BvlLQlOPHY=; b=grTXHHBhOlEu245DhPxZVWD8VZ9mmh67NtSgsvfL8zQcdCfFBKbO5m+LAgBM7WD+8C XPr67xpeGRZD7X0NIwrKiAW95HHQXJps9NyNn/OjacKFnLB6U8A0bUL66fSCqiK2mwdl JRUpY2TTEzaG5SriJ1A8K2fbBFOfwLKATdGqorG42GATa4/AlL5vj0dKST3Oa/KPZ7/w TSiQAsANI/CwyhFpvxQWIZzPhfn6lvb3XUT48FthY4HIcSOlunrwcEceslrJp9azeg56 Rcqx56fC6/3R4kO4r8qylZxSGrkg0weic8LzubhvMpQ3NBLR9EVmjQI5oX1c5SWgu84N nT3A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682592722; x=1685184722; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=j7spBiryByRXoOgHwqaNuAV486OshQIV2BvlLQlOPHY=; b=GEHgNlgkXGPZiM8UWQ+YpRF1tTiGcPkTIpdF+/NYcpQpDAtYBX2iJVwlZOg9ItBggn Ex8BtcMXx3tzWU/sS8ii2ZOzQsauCNSDEYCpT5XK8kIssKgAoQmWxtvDdzPIMILRtj5P 48K1eg99ZlVI4hP70Cl5RlvNPwlBQbeLzC2vDsGX3niqHeTbyoHYL8lcifD7YGXT8suZ 8nL9in4PpKSCigv/DxRntq26VVunJs+eGcmCLAnXZtL8faHXvvEDcJ53pXfVEQYTX3gR jHyV4X5SQLX6dQSR7VhKUTGad6n2tuJ8v5GSR1GgXR74gHqZY6RIQ2E2b0QBanvjf1SS 8X0A== X-Gm-Message-State: AC+VfDyw0PpMjBFnTq2DC7G30rRRfIz835TII5IY2a+tqR1wc7rAKMCR Tu6/x8ZpPBAbJOBoi/syf26UqR5yU0gMxunWySB2E5C5 X-Google-Smtp-Source: ACHHUZ6H7+ZpGK8rOPAiqw/Kb3BmHHTcscZRS265zZ3gKPcHjYPVcY2AHe7PltZvIq4YCqMZl6FPXuaL7VCn37I6l1U= X-Received: by 2002:a2e:80c3:0:b0:2a7:7730:9ed with SMTP id r3-20020a2e80c3000000b002a7773009edmr477272ljg.12.1682592721757; Thu, 27 Apr 2023 03:52:01 -0700 (PDT) MIME-Version: 1.0 References: <20230424213011.528181-1-apinski@marvell.com> <20230424213011.528181-2-apinski@marvell.com> In-Reply-To: <20230424213011.528181-2-apinski@marvell.com> From: Richard Biener Date: Thu, 27 Apr 2023 12:50:32 +0200 Message-ID: Subject: Re: [PATCH 1/7] PHIOPT: Split out store elimination from phiopt To: Andrew Pinski Cc: gcc-patches@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-7.7 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,T_SCC_BODY_TEXT_LINE 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 Mon, Apr 24, 2023 at 11:31=E2=80=AFPM Andrew Pinski via Gcc-patches wrote: > > Since the last cleanups, it made easier to see > that we should split out the store elimination > worker from tree_ssa_phiopt_worker function. > > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. OK. > gcc/ChangeLog: > > * tree-ssa-phiopt.cc (tree_ssa_phiopt_worker): Remove > do_store_elim argument and split that part out to ... > (store_elim_worker): This new function. > (pass_cselim::execute): Call store_elim_worker. > (pass_phiopt::execute): Update call to tree_ssa_phiopt_worker. > --- > gcc/tree-ssa-phiopt.cc | 180 ++++++++++++++++++++++++++++------------- > 1 file changed, 126 insertions(+), 54 deletions(-) > > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc > index 4a3ab8efb71..7f47b32576b 100644 > --- a/gcc/tree-ssa-phiopt.cc > +++ b/gcc/tree-ssa-phiopt.cc > @@ -104,27 +104,19 @@ single_non_singleton_phi_for_edges (gimple_seq seq,= edge e0, edge e1) > return phi; > } > > -/* The core routine of conditional store replacement and normal > - phi optimizations. Both share much of the infrastructure in how > - to match applicable basic block patterns. DO_STORE_ELIM is true > - when we want to do conditional store replacement, false otherwise. > +/* The core routine of phi optimizations. > DO_HOIST_LOADS is true when we want to hoist adjacent loads out > of diamond control flow patterns, false otherwise. */ > static unsigned int > -tree_ssa_phiopt_worker (bool do_store_elim, bool do_hoist_loads, bool ea= rly_p) > +tree_ssa_phiopt_worker (bool do_hoist_loads, bool early_p) > { > basic_block bb; > basic_block *bb_order; > unsigned n, i; > bool cfgchanged =3D false; > - hash_set *nontrap =3D 0; > > calculate_dominance_info (CDI_DOMINATORS); > > - if (do_store_elim) > - /* Calculate the set of non-trapping memory accesses. */ > - nontrap =3D get_non_trapping (); > - > /* Search every basic block for COND_EXPR we may be able to optimize. > > We walk the blocks in order that guarantees that a block with > @@ -148,7 +140,7 @@ tree_ssa_phiopt_worker (bool do_store_elim, bool do_h= oist_loads, bool early_p) > /* Check to see if the last statement is a GIMPLE_COND. */ > gcond *cond_stmt =3D safe_dyn_cast (*gsi_last_bb (bb)); > if (!cond_stmt) > - continue; > + continue; > > e1 =3D EDGE_SUCC (bb, 0); > bb1 =3D e1->dest; > @@ -158,12 +150,12 @@ tree_ssa_phiopt_worker (bool do_store_elim, bool do= _hoist_loads, bool early_p) > /* We cannot do the optimization on abnormal edges. */ > if ((e1->flags & EDGE_ABNORMAL) !=3D 0 > || (e2->flags & EDGE_ABNORMAL) !=3D 0) > - continue; > + continue; > > /* If either bb1's succ or bb2 or bb2's succ is non NULL. */ > if (EDGE_COUNT (bb1->succs) =3D=3D 0 > || EDGE_COUNT (bb2->succs) =3D=3D 0) > - continue; > + continue; > > /* Find the bb which is the fall through to the other. */ > if (EDGE_SUCC (bb1, 0)->dest =3D=3D bb2) > @@ -192,39 +184,6 @@ tree_ssa_phiopt_worker (bool do_store_elim, bool do_= hoist_loads, bool early_p) > || (e1->flags & EDGE_FALLTHRU) =3D=3D 0) > continue; > > - if (do_store_elim) > - { > - if (diamond_p) > - { > - basic_block bb3 =3D e1->dest; > - > - /* Only handle sinking of store from 2 bbs only, > - The middle bbs don't need to come from the > - if always since we are sinking rather than > - hoisting. */ > - if (EDGE_COUNT (bb3->preds) !=3D 2) > - continue; > - if (cond_if_else_store_replacement (bb1, bb2, bb3)) > - cfgchanged =3D true; > - continue; > - } > - > - /* Also make sure that bb1 only have one predecessor and that i= t > - is bb. */ > - if (!single_pred_p (bb1) > - || single_pred (bb1) !=3D bb) > - continue; > - > - /* bb1 is the middle block, bb2 the join block, bb the split bl= ock, > - e1 the fallthrough edge from bb1 to bb2. We can't do the > - optimization if the join block has more than two predecessor= s. */ > - if (EDGE_COUNT (bb2->preds) > 2) > - continue; > - if (cond_store_replacement (bb1, bb2, e1, e2, nontrap)) > - cfgchanged =3D true; > - continue; > - } > - > if (diamond_p) > { > basic_block bb3 =3D e1->dest; > @@ -322,18 +281,132 @@ tree_ssa_phiopt_worker (bool do_store_elim, bool d= o_hoist_loads, bool early_p) > > free (bb_order); > > - if (do_store_elim) > - delete nontrap; > + if (cfgchanged) > + return TODO_cleanup_cfg; > + return 0; > +} > + > +/* The core routine of conditional store replacement. */ > +static unsigned int > +store_elim_worker (void) > +{ > + basic_block bb; > + basic_block *bb_order; > + unsigned n, i; > + bool cfgchanged =3D false; > + hash_set *nontrap =3D 0; > + > + calculate_dominance_info (CDI_DOMINATORS); > + > + /* Calculate the set of non-trapping memory accesses. */ > + nontrap =3D get_non_trapping (); > + > + /* Search every basic block for COND_EXPR we may be able to optimize. > + > + We walk the blocks in order that guarantees that a block with > + a single predecessor is processed before the predecessor. > + This ensures that we collapse inner ifs before visiting the > + outer ones, and also that we do not try to visit a removed > + block. */ > + bb_order =3D single_pred_before_succ_order (); > + n =3D n_basic_blocks_for_fn (cfun) - NUM_FIXED_BLOCKS; > + > + for (i =3D 0; i < n; i++) > + { > + basic_block bb1, bb2; > + edge e1, e2; > + bool diamond_p =3D false; > + > + bb =3D bb_order[i]; > + > + /* Check to see if the last statement is a GIMPLE_COND. */ > + gcond *cond_stmt =3D safe_dyn_cast (*gsi_last_bb (bb)); > + if (!cond_stmt) > + continue; > + > + e1 =3D EDGE_SUCC (bb, 0); > + bb1 =3D e1->dest; > + e2 =3D EDGE_SUCC (bb, 1); > + bb2 =3D e2->dest; > + > + /* We cannot do the optimization on abnormal edges. */ > + if ((e1->flags & EDGE_ABNORMAL) !=3D 0 > + || (e2->flags & EDGE_ABNORMAL) !=3D 0) > + continue; > + > + /* If either bb1's succ or bb2 or bb2's succ is non NULL. */ > + if (EDGE_COUNT (bb1->succs) =3D=3D 0 > + || EDGE_COUNT (bb2->succs) =3D=3D 0) > + continue; > + > + /* Find the bb which is the fall through to the other. */ > + if (EDGE_SUCC (bb1, 0)->dest =3D=3D bb2) > + ; > + else if (EDGE_SUCC (bb2, 0)->dest =3D=3D bb1) > + { > + std::swap (bb1, bb2); > + std::swap (e1, e2); > + } > + else if (EDGE_SUCC (bb1, 0)->dest =3D=3D EDGE_SUCC (bb2, 0)->dest > + && single_succ_p (bb2)) > + { > + diamond_p =3D true; > + e2 =3D EDGE_SUCC (bb2, 0); > + /* Make sure bb2 is just a fall through. */ > + if ((e2->flags & EDGE_FALLTHRU) =3D=3D 0) > + continue; > + } > + else > + continue; > + > + e1 =3D EDGE_SUCC (bb1, 0); > + > + /* Make sure that bb1 is just a fall through. */ > + if (!single_succ_p (bb1) > + || (e1->flags & EDGE_FALLTHRU) =3D=3D 0) > + continue; > + > + if (diamond_p) > + { > + basic_block bb3 =3D e1->dest; > + > + /* Only handle sinking of store from 2 bbs only, > + The middle bbs don't need to come from the > + if always since we are sinking rather than > + hoisting. */ > + if (EDGE_COUNT (bb3->preds) !=3D 2) > + continue; > + if (cond_if_else_store_replacement (bb1, bb2, bb3)) > + cfgchanged =3D true; > + continue; > + } > + > + /* Also make sure that bb1 only have one predecessor and that it > + is bb. */ > + if (!single_pred_p (bb1) > + || single_pred (bb1) !=3D bb) > + continue; > + > + /* bb1 is the middle block, bb2 the join block, bb the split block= , > + e1 the fallthrough edge from bb1 to bb2. We can't do the > + optimization if the join block has more than two predecessors. = */ > + if (EDGE_COUNT (bb2->preds) > 2) > + continue; > + if (cond_store_replacement (bb1, bb2, e1, e2, nontrap)) > + cfgchanged =3D true; > + } > + > + free (bb_order); > + > + delete nontrap; > /* If the CFG has changed, we should cleanup the CFG. */ > - if (cfgchanged && do_store_elim) > + if (cfgchanged) > { > /* In cond-store replacement we have added some loads on edges > and new VOPS (as we moved the store, and created a load). */ > gsi_commit_edge_inserts (); > return TODO_cleanup_cfg | TODO_update_ssa_only_virtuals; > } > - else if (cfgchanged) > - return TODO_cleanup_cfg; > return 0; > } > > @@ -4257,8 +4330,7 @@ public: > bool gate (function *) final override { return flag_ssa_phiopt; } > unsigned int execute (function *) final override > { > - return tree_ssa_phiopt_worker (false, > - !early_p ? gate_hoist_loads () : fal= se, > + return tree_ssa_phiopt_worker (!early_p ? gate_hoist_loads () : fa= lse, > early_p); > } > > @@ -4360,7 +4432,7 @@ pass_cselim::execute (function *) > An interfacing issue of find_data_references_in_bb. */ > loop_optimizer_init (LOOPS_NORMAL); > scev_initialize (); > - todo =3D tree_ssa_phiopt_worker (true, false, false); > + todo =3D store_elim_worker (); > scev_finalize (); > loop_optimizer_finalize (); > return todo; > -- > 2.39.1 >