From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi1-x22c.google.com (mail-oi1-x22c.google.com [IPv6:2607:f8b0:4864:20::22c]) by sourceware.org (Postfix) with ESMTPS id 7A59C3853828 for ; Tue, 29 Jun 2021 19:11:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7A59C3853828 Received: by mail-oi1-x22c.google.com with SMTP id w127so27449001oig.12 for ; Tue, 29 Jun 2021 12:11:03 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=0Avx6g7BmJA8qRH4GU+hgw0IhVAbJ7g0koom4lGQ5Wc=; b=mtHNdqLmDDl5zq6dF9Lwf50x8/4EnUT0ajdG8WIJmi3kB3QRPZmTOgwlDM/SZdK4yC sLtYwVc4izHZJVcJrrly0lM+szAbGJ1EWo0yCIlgEbJcnEbiIBUF8416pn+tyCNTJPw0 5KYwsYoqwrV+9PbQvk28CjVilnig8RaiPTuWnZyc8lpBIRivrkapQMDwZx6YQsMhqt54 a3hJNeV5Yc57Ar0G1kZETM8j+3yyfRxzWoYNYFtIA1saraVx6lci6lwblKbPw6aaczZI Jg3WtxtMsznOXsuzfRT2Mo4fVMs/lKhqE9WXX5wwqYFDnHKx/I6fV3e8JWpHiTr5vr5J TxlQ== X-Gm-Message-State: AOAM530+No6DpMqlkgUm1RD8pB/kExqXaKifX+MEP2gs60GAVHOebGh0 2LTqzZUJ/GgoEKerPeIo+9ffRJ6wCJ8QKQ== X-Google-Smtp-Source: ABdhPJxBwtOM6y4HEZu73yIjToI210pHsQgKR2g1Ffg+jEM1iAq6aRVYJY4RMPWA0zKxFaPkkSuCfA== X-Received: by 2002:a05:6808:114b:: with SMTP id u11mr22451959oiu.167.1624993862652; Tue, 29 Jun 2021 12:11:02 -0700 (PDT) Received: from [192.168.0.41] (97-118-105-195.hlrn.qwest.net. [97.118.105.195]) by smtp.gmail.com with ESMTPSA id d18sm4253162otu.71.2021.06.29.12.11.01 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 29 Jun 2021 12:11:02 -0700 (PDT) Subject: Re: [PATCH 2/4] Allow match-and-simplified phiopt to run in early phiopt To: apinski@marvell.com, gcc-patches@gcc.gnu.org References: <1624836300-23553-1-git-send-email-apinski@marvell.com> <1624836300-23553-3-git-send-email-apinski@marvell.com> From: Martin Sebor Message-ID: Date: Tue, 29 Jun 2021 13:11:01 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: <1624836300-23553-3-git-send-email-apinski@marvell.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Tue, 29 Jun 2021 19:11:07 -0000 On 6/27/21 5:24 PM, apinski--- via Gcc-patches wrote: > From: Andrew Pinski > > To move a few things more to match-and-simplify from phiopt, > we need to allow match_simplify_replacement to run in early > phiopt. To do this we add a replacement for gimple_simplify > that is explictly for phiopt. > > OK? Bootstrapped and tested on x86_64-linux-gnu with no > regressions. > > gcc/ChangeLog: > > * tree-ssa-phiopt.c (match_simplify_replacement): > Add early_p argument. Call gimple_simplify_phiopt > instead of gimple_simplify. > (tree_ssa_phiopt_worker): Update call to > match_simplify_replacement and allow unconditionally. > (phiopt_early_allow): New function. > (gimple_simplify_phiopt): New function. > --- > gcc/tree-ssa-phiopt.c | 89 ++++++++++++++++++++++++++++++++++--------- > 1 file changed, 70 insertions(+), 19 deletions(-) > > diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c > index ab12e85569d..17bc597851b 100644 > --- a/gcc/tree-ssa-phiopt.c > +++ b/gcc/tree-ssa-phiopt.c > @@ -50,12 +50,13 @@ along with GCC; see the file COPYING3. If not see > #include "gimple-fold.h" > #include "internal-fn.h" > #include "gimple-range.h" > +#include "gimple-match.h" > > static unsigned int tree_ssa_phiopt_worker (bool, bool, bool); > static bool two_value_replacement (basic_block, basic_block, edge, gphi *, > tree, tree); > static bool match_simplify_replacement (basic_block, basic_block, > - edge, edge, gphi *, tree, tree); > + edge, edge, gphi *, tree, tree, bool); > static gphi *factor_out_conditional_conversion (edge, edge, gphi *, tree, tree, > gimple *); > static int value_replacement (basic_block, basic_block, > @@ -345,9 +346,9 @@ tree_ssa_phiopt_worker (bool do_store_elim, bool do_hoist_loads, bool early_p) > /* Do the replacement of conditional if it can be done. */ > if (!early_p && two_value_replacement (bb, bb1, e2, phi, arg0, arg1)) > cfgchanged = true; > - else if (!early_p > - && match_simplify_replacement (bb, bb1, e1, e2, phi, > - arg0, arg1)) > + else if (match_simplify_replacement (bb, bb1, e1, e2, phi, > + arg0, arg1, > + early_p)) > cfgchanged = true; > else if (abs_replacement (bb, bb1, e1, e2, phi, arg0, arg1)) > cfgchanged = true; > @@ -811,6 +812,67 @@ two_value_replacement (basic_block cond_bb, basic_block middle_bb, > return true; > } > > +/* Return TRUE if CODE should be allowed during early phiopt. > + Currently this is to allow MIN/MAX and ABS/NEGATE. */ > +static bool > +phiopt_early_allow (enum tree_code code) > +{ > + switch (code) > + { > + case MIN_EXPR: > + case MAX_EXPR: > + case ABS_EXPR: > + case ABSU_EXPR: > + case NEGATE_EXPR: > + case SSA_NAME: > + return true; > + default: > + return false; > + } > +} > + > +/* gimple_simplify_phiopt is like gimple_simplify but designed for PHIOPT. > + Return NULL if nothing can be simplified or the resulting simplified value > + with parts pushed if EARLY_P was true. Also rejects non allowed tree code > + if EARLY_P is set. > + Takes the comparison from COMP_STMT and two args, ARG0 and ARG1 and tries > + to simplify CMP ? ARG0 : ARG1. */ > +static tree > +gimple_simplify_phiopt (bool early_p, tree type, gimple *comp_stmt, > + tree arg0, tree arg1, > + gimple_seq *seq) > +{ > + tree result; > + enum tree_code comp_code = gimple_cond_code (comp_stmt); > + location_t loc = gimple_location (comp_stmt); > + tree cmp0 = gimple_cond_lhs (comp_stmt); > + tree cmp1 = gimple_cond_rhs (comp_stmt); > + /* To handle special cases like floating point comparison, it is easier and > + less error-prone to build a tree and gimplify it on the fly though it is > + less efficient. > + Don't use fold_build2 here as that might create (bool)a instead of just > + "a != 0". */ > + tree cond = build2_loc (loc, comp_code, boolean_type_node, > + cmp0, cmp1); > + gimple_match_op op (gimple_match_cond::UNCOND, > + COND_EXPR, type, cond, arg0, arg1); > + > + if (op.resimplify (early_p ? NULL : seq, follow_all_ssa_edges)) > + { > + /* Early we want only to allow some generated tree codes. */ > + if (!early_p > + || op.code.is_tree_code () > + || phiopt_early_allow ((tree_code)op.code)) > + { > + result = maybe_push_res_to_seq (&op, seq); > + if (result) > + return result; It looks to me like the last if statement is redundant and could be replaced by return maybe_push_res_to_seq (&op, seq); thus also making the result variable redundant, further simplifying the code. Martin > + } > + } > + > + return NULL; > +} > + > /* The function match_simplify_replacement does the main work of doing the > replacement using match and simplify. Return true if the replacement is done. > Otherwise return false. > @@ -820,10 +882,9 @@ two_value_replacement (basic_block cond_bb, basic_block middle_bb, > static bool > match_simplify_replacement (basic_block cond_bb, basic_block middle_bb, > edge e0, edge e1, gphi *phi, > - tree arg0, tree arg1) > + tree arg0, tree arg1, bool early_p) > { > gimple *stmt; > - tree cond; > gimple_stmt_iterator gsi; > edge true_edge, false_edge; > gimple_seq seq = NULL; > @@ -884,15 +945,6 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb, > > stmt = last_stmt (cond_bb); > > - /* To handle special cases like floating point comparison, it is easier and > - less error-prone to build a tree and gimplify it on the fly though it is > - less efficient. > - Don't use fold_build2 here as that might create (bool)a instead of just > - "a != 0". */ > - cond = build2_loc (gimple_location (stmt), > - gimple_cond_code (stmt), boolean_type_node, > - gimple_cond_lhs (stmt), gimple_cond_rhs (stmt)); > - > /* We need to know which is the true edge and which is the false > edge so that we know when to invert the condition below. */ > extract_true_false_edges_from_block (cond_bb, &true_edge, &false_edge); > @@ -900,10 +952,9 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb, > std::swap (arg0, arg1); > > tree type = TREE_TYPE (gimple_phi_result (phi)); > - result = gimple_simplify (COND_EXPR, type, > - cond, > - arg0, arg1, > - &seq, NULL); > + result = gimple_simplify_phiopt (early_p, type, stmt, > + arg0, arg1, > + &seq); > if (!result) > return false; > >