From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x532.google.com (mail-ed1-x532.google.com [IPv6:2a00:1450:4864:20::532]) by sourceware.org (Postfix) with ESMTPS id 076AC3857C5F for ; Thu, 18 Nov 2021 14:27:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 076AC3857C5F Received: by mail-ed1-x532.google.com with SMTP id z5so27887115edd.3 for ; Thu, 18 Nov 2021 06:27:22 -0800 (PST) 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; bh=usxLKDNhV8LvOJtNcB5lLt+Qv+E6Tg98sICYTY4Pf3Y=; b=lCp0Hnr0/BL32wxNfH6Q5NDZ12YP21y5B5MZuFnSrzeuHzY6t9tKiZhdOTKispFjVN Lpp17IKk8KkxIbOdJR2u1lnEhMU+iT4sS3dsQOd5Z4e65Q0a+TXRxVsNa5o1zy9scoRh +ZXlkeI8ai7rzequlXe1tTfwKIZTkzor2WogdqmsAR1A9mrN3yep/sbw2FmfMB8hhlJ9 /SvIlT1CvhAWHxxSf+ClCGnYLr5pnyhWbAL9w7HoRZuAv0jeUHIqJdbIPgNDiNkrPu+k v6ZJ0GGpjyYPrNRRcQp3HOAQA0mCrXlJ+boIUzVW2eZ63DQwQtXXn2fgjeGxGqfWBUAM hWXg== X-Gm-Message-State: AOAM530v3h2xhKIWmfEj4zoIJAa85DvteoKuiklkOjbtlhQm5s2xjuNA vuMizHm1LL0yvGPtCuxe+Ie4kwIh7ypLY1uohfk= X-Google-Smtp-Source: ABdhPJxVTV1fHB6yboBdh4i15tRoeHhCVcWdnuG3QorCK6mGxOcY2exaO0ZG51tJNC+LZnKAOZPU6Ar3VSLVJqRcI/k= X-Received: by 2002:aa7:d510:: with SMTP id y16mr12244611edq.338.1637245641939; Thu, 18 Nov 2021 06:27:21 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Richard Biener Date: Thu, 18 Nov 2021 15:27:10 +0100 Message-ID: Subject: Re: [PATCH 2/5] gimple-match: Add a gimple_extract_op function To: Richard Biener , GCC Patches , Richard Sandiford Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-8.3 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, 18 Nov 2021 14:27:26 -0000 On Tue, Nov 16, 2021 at 4:51 PM Richard Sandiford wrote: > > Richard Biener writes: > > On Wed, Nov 10, 2021 at 1:46 PM Richard Sandiford via Gcc-patches > > wrote: > >> > >> code_helper and gimple_match_op seem like generally useful ways > >> of summing up a gimple_assign or gimple_call (or gimple_cond). > >> This patch adds a gimple_extract_op function that can be used > >> for that. > >> > >> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? > >> > >> Richard > >> > >> > >> gcc/ > >> * gimple-match.h (gimple_extract_op): Declare. > >> * gimple-match.c (gimple_extract): New function, extracted from... > >> (gimple_simplify): ...here. > >> (gimple_extract_op): New function. > >> --- > >> gcc/gimple-match-head.c | 261 +++++++++++++++++++++++----------------- > >> gcc/gimple-match.h | 1 + > >> 2 files changed, 149 insertions(+), 113 deletions(-) > >> > >> diff --git a/gcc/gimple-match-head.c b/gcc/gimple-match-head.c > >> index 9d88b2f8551..4c6e0883ba4 100644 > >> --- a/gcc/gimple-match-head.c > >> +++ b/gcc/gimple-match-head.c > >> @@ -890,12 +890,29 @@ try_conditional_simplification (internal_fn ifn, gimple_match_op *res_op, > >> return true; > >> } > >> > >> -/* The main STMT based simplification entry. It is used by the fold_stmt > >> - and the fold_stmt_to_constant APIs. */ > >> +/* Common subroutine of gimple_extract_op and gimple_simplify. Try to > >> + describe STMT in RES_OP. Return: > >> > >> -bool > >> -gimple_simplify (gimple *stmt, gimple_match_op *res_op, gimple_seq *seq, > >> - tree (*valueize)(tree), tree (*top_valueize)(tree)) > >> + - -1 if extraction failed > >> + - otherwise, 0 if no simplification should take place > >> + - otherwise, the number of operands for a GIMPLE_ASSIGN or GIMPLE_COND > >> + - otherwise, -2 for a GIMPLE_CALL > >> + > >> + Before recording an operand, call: > >> + > >> + - VALUEIZE_CONDITION for a COND_EXPR condition > >> + - VALUEIZE_NAME if the rhs of a GIMPLE_ASSIGN is an SSA_NAME > > > > I think at least VALUEIZE_NAME is unnecessary, see below > > Yeah, it's unnecessary. The idea was to (try to) ensure that > gimple_simplify keeps all the microoptimisations that it had > previously. This includes open-coding do_valueize for SSA_NAMEs > and jumping straight to the right gimplify_resimplifyN routine > when the number of operands is already known. > > (The two calls to gimple_extract<> produce different functions > that ought to get inlined into their single callers. A lot of the > jumps should then be threaded.) > > I can drop all that if you don't think it's worth it though. > Just wanted to double-check first. Yes, I'd prefer the simpler and more understandable variant. Richard. > > Thanks, > Richard > > >> + - VALUEIZE_OP for every other top-level operand > >> + > >> + Each routine takes a tree argument and returns a tree. */ > >> + > >> +template >> + typename ValueizeName> > >> +inline int > >> +gimple_extract (gimple *stmt, gimple_match_op *res_op, > >> + ValueizeOp valueize_op, > >> + ValueizeCondition valueize_condition, > >> + ValueizeName valueize_name) > >> { > >> switch (gimple_code (stmt)) > >> { > >> @@ -911,100 +928,53 @@ gimple_simplify (gimple *stmt, gimple_match_op *res_op, gimple_seq *seq, > >> || code == VIEW_CONVERT_EXPR) > >> { > >> tree op0 = TREE_OPERAND (gimple_assign_rhs1 (stmt), 0); > >> - bool valueized = false; > >> - op0 = do_valueize (op0, top_valueize, valueized); > >> - res_op->set_op (code, type, op0); > >> - return (gimple_resimplify1 (seq, res_op, valueize) > >> - || valueized); > >> + res_op->set_op (code, type, valueize_op (op0)); > >> + return 1; > >> } > >> else if (code == BIT_FIELD_REF) > >> { > >> tree rhs1 = gimple_assign_rhs1 (stmt); > >> - tree op0 = TREE_OPERAND (rhs1, 0); > >> - bool valueized = false; > >> - op0 = do_valueize (op0, top_valueize, valueized); > >> + tree op0 = valueize_op (TREE_OPERAND (rhs1, 0)); > >> res_op->set_op (code, type, op0, > >> TREE_OPERAND (rhs1, 1), > >> TREE_OPERAND (rhs1, 2), > >> REF_REVERSE_STORAGE_ORDER (rhs1)); > >> - if (res_op->reverse) > >> - return valueized; > >> - return (gimple_resimplify3 (seq, res_op, valueize) > >> - || valueized); > >> + return res_op->reverse ? 0 : 3; > >> } > >> - else if (code == SSA_NAME > >> - && top_valueize) > >> + else if (code == SSA_NAME) > >> { > >> tree op0 = gimple_assign_rhs1 (stmt); > >> - tree valueized = top_valueize (op0); > >> + tree valueized = valueize_name (op0); > >> if (!valueized || op0 == valueized) > >> - return false; > >> + return -1; > >> res_op->set_op (TREE_CODE (op0), type, valueized); > >> - return true; > >> + return 0; > > > > So the old code in an obfuscated way just knowed nothing simplifies > > on the plain not valueized name but returned true when valueization > > changed the stmt. So I'd expect > > > > tree valueized = valueize_op (op0); > > res_op->set_op (TREE_CODE (op0), type, valueized); > > return 0; > > > > here and the gimple_simplify caller returning 'valueized'. I think > > that the old code treated a NULL top_valueize () as "fail" is just > > premature optimization without any effect. > > > >> } > >> break; > >> case GIMPLE_UNARY_RHS: > >> { > >> tree rhs1 = gimple_assign_rhs1 (stmt); > >> - bool valueized = false; > >> - rhs1 = do_valueize (rhs1, top_valueize, valueized); > >> - res_op->set_op (code, type, rhs1); > >> - return (gimple_resimplify1 (seq, res_op, valueize) > >> - || valueized); > >> + res_op->set_op (code, type, valueize_op (rhs1)); > >> + return 1; > >> } > >> case GIMPLE_BINARY_RHS: > >> { > >> - tree rhs1 = gimple_assign_rhs1 (stmt); > >> - tree rhs2 = gimple_assign_rhs2 (stmt); > >> - bool valueized = false; > >> - rhs1 = do_valueize (rhs1, top_valueize, valueized); > >> - rhs2 = do_valueize (rhs2, top_valueize, valueized); > >> + tree rhs1 = valueize_op (gimple_assign_rhs1 (stmt)); > >> + tree rhs2 = valueize_op (gimple_assign_rhs2 (stmt)); > >> res_op->set_op (code, type, rhs1, rhs2); > >> - return (gimple_resimplify2 (seq, res_op, valueize) > >> - || valueized); > >> + return 2; > >> } > >> case GIMPLE_TERNARY_RHS: > >> { > >> - bool valueized = false; > >> tree rhs1 = gimple_assign_rhs1 (stmt); > >> - /* If this is a COND_EXPR first try to simplify an > >> - embedded GENERIC condition. */ > >> - if (code == COND_EXPR) > >> - { > >> - if (COMPARISON_CLASS_P (rhs1)) > >> - { > >> - tree lhs = TREE_OPERAND (rhs1, 0); > >> - tree rhs = TREE_OPERAND (rhs1, 1); > >> - lhs = do_valueize (lhs, top_valueize, valueized); > >> - rhs = do_valueize (rhs, top_valueize, valueized); > >> - gimple_match_op res_op2 (res_op->cond, TREE_CODE (rhs1), > >> - TREE_TYPE (rhs1), lhs, rhs); > >> - if ((gimple_resimplify2 (seq, &res_op2, valueize) > >> - || valueized) > >> - && res_op2.code.is_tree_code ()) > >> - { > >> - valueized = true; > >> - if (TREE_CODE_CLASS ((enum tree_code) res_op2.code) > >> - == tcc_comparison) > >> - rhs1 = build2 (res_op2.code, TREE_TYPE (rhs1), > >> - res_op2.ops[0], res_op2.ops[1]); > >> - else if (res_op2.code == SSA_NAME > >> - || res_op2.code == INTEGER_CST > >> - || res_op2.code == VECTOR_CST) > >> - rhs1 = res_op2.ops[0]; > >> - else > >> - valueized = false; > >> - } > >> - } > >> - } > >> - tree rhs2 = gimple_assign_rhs2 (stmt); > >> - tree rhs3 = gimple_assign_rhs3 (stmt); > >> - rhs1 = do_valueize (rhs1, top_valueize, valueized); > >> - rhs2 = do_valueize (rhs2, top_valueize, valueized); > >> - rhs3 = do_valueize (rhs3, top_valueize, valueized); > >> + if (code == COND_EXPR && COMPARISON_CLASS_P (rhs1)) > >> + rhs1 = valueize_condition (rhs1); > >> + else > >> + rhs1 = valueize_op (rhs1); > >> + tree rhs2 = valueize_op (gimple_assign_rhs2 (stmt)); > >> + tree rhs3 = valueize_op (gimple_assign_rhs3 (stmt)); > >> res_op->set_op (code, type, rhs1, rhs2, rhs3); > >> - return (gimple_resimplify3 (seq, res_op, valueize) > >> - || valueized); > >> + return 3; > >> } > >> default: > >> gcc_unreachable (); > >> @@ -1018,7 +988,6 @@ gimple_simplify (gimple *stmt, gimple_match_op *res_op, gimple_seq *seq, > >> && gimple_call_num_args (stmt) >= 1 > >> && gimple_call_num_args (stmt) <= 5) > >> { > >> - bool valueized = false; > >> combined_fn cfn; > >> if (gimple_call_internal_p (stmt)) > >> cfn = as_combined_fn (gimple_call_internal_fn (stmt)); > >> @@ -1026,17 +995,17 @@ gimple_simplify (gimple *stmt, gimple_match_op *res_op, gimple_seq *seq, > >> { > >> tree fn = gimple_call_fn (stmt); > >> if (!fn) > >> - return false; > >> + return -1; > >> > >> - fn = do_valueize (fn, top_valueize, valueized); > >> + fn = valueize_op (fn); > >> if (TREE_CODE (fn) != ADDR_EXPR > >> || TREE_CODE (TREE_OPERAND (fn, 0)) != FUNCTION_DECL) > >> - return false; > >> + return -1; > >> > >> tree decl = TREE_OPERAND (fn, 0); > >> if (DECL_BUILT_IN_CLASS (decl) != BUILT_IN_NORMAL > >> || !gimple_builtin_call_types_compatible_p (stmt, decl)) > >> - return false; > >> + return -1; > >> > >> cfn = as_combined_fn (DECL_FUNCTION_CODE (decl)); > >> } > >> @@ -1044,56 +1013,122 @@ gimple_simplify (gimple *stmt, gimple_match_op *res_op, gimple_seq *seq, > >> unsigned int num_args = gimple_call_num_args (stmt); > >> res_op->set_op (cfn, TREE_TYPE (gimple_call_lhs (stmt)), num_args); > >> for (unsigned i = 0; i < num_args; ++i) > >> - { > >> - tree arg = gimple_call_arg (stmt, i); > >> - res_op->ops[i] = do_valueize (arg, top_valueize, valueized); > >> - } > >> - if (internal_fn_p (cfn) > >> - && try_conditional_simplification (as_internal_fn (cfn), > >> - res_op, seq, valueize)) > >> - return true; > >> - switch (num_args) > >> - { > >> - case 1: > >> - return (gimple_resimplify1 (seq, res_op, valueize) > >> - || valueized); > >> - case 2: > >> - return (gimple_resimplify2 (seq, res_op, valueize) > >> - || valueized); > >> - case 3: > >> - return (gimple_resimplify3 (seq, res_op, valueize) > >> - || valueized); > >> - case 4: > >> - return (gimple_resimplify4 (seq, res_op, valueize) > >> - || valueized); > >> - case 5: > >> - return (gimple_resimplify5 (seq, res_op, valueize) > >> - || valueized); > >> - default: > >> - gcc_unreachable (); > >> - } > >> + res_op->ops[i] = valueize_op (gimple_call_arg (stmt, i)); > >> + return -2; > >> } > >> break; > >> > >> case GIMPLE_COND: > >> { > >> - tree lhs = gimple_cond_lhs (stmt); > >> - tree rhs = gimple_cond_rhs (stmt); > >> - bool valueized = false; > >> - lhs = do_valueize (lhs, top_valueize, valueized); > >> - rhs = do_valueize (rhs, top_valueize, valueized); > >> + tree lhs = valueize_op (gimple_cond_lhs (stmt)); > >> + tree rhs = valueize_op (gimple_cond_rhs (stmt)); > >> res_op->set_op (gimple_cond_code (stmt), boolean_type_node, lhs, rhs); > >> - return (gimple_resimplify2 (seq, res_op, valueize) > >> - || valueized); > >> + return 2; > >> } > >> > >> default: > >> break; > >> } > >> > >> - return false; > >> + return -1; > >> } > >> > >> +/* Try to describe STMT in RES_OP, returning true on success. > >> + For GIMPLE_CONDs, describe the condition that is being tested. > >> + For GIMPLE_ASSIGNs, describe the rhs of the assignment. > >> + For GIMPLE_CALLs, describe the call. */ > >> + > >> +bool > >> +gimple_extract_op (gimple *stmt, gimple_match_op *res_op) > >> +{ > >> + auto nop = [](tree op) { return op; }; > >> + return gimple_extract (stmt, res_op, nop, nop, nop) != -1; > >> +} > >> + > >> +/* The main STMT based simplification entry. It is used by the fold_stmt > >> + and the fold_stmt_to_constant APIs. */ > >> + > >> +bool > >> +gimple_simplify (gimple *stmt, gimple_match_op *res_op, gimple_seq *seq, > >> + tree (*valueize)(tree), tree (*top_valueize)(tree)) > >> +{ > >> + bool valueized = false; > >> + auto valueize_op = [&](tree op) > >> + { > >> + return do_valueize (op, top_valueize, valueized); > >> + }; > >> + auto valueize_condition = [&](tree op) -> tree > >> + { > >> + bool cond_valueized = false; > >> + tree lhs = do_valueize (TREE_OPERAND (op, 0), top_valueize, > >> + cond_valueized); > >> + tree rhs = do_valueize (TREE_OPERAND (op, 1), top_valueize, > >> + cond_valueized); > >> + gimple_match_op res_op2 (res_op->cond, TREE_CODE (op), > >> + TREE_TYPE (op), lhs, rhs); > >> + if ((gimple_resimplify2 (seq, &res_op2, valueize) > >> + || cond_valueized) > >> + && res_op2.code.is_tree_code ()) > >> + { > >> + if (TREE_CODE_CLASS ((tree_code) res_op2.code) == tcc_comparison) > >> + { > >> + valueized = true; > >> + return build2 (res_op2.code, TREE_TYPE (op), > >> + res_op2.ops[0], res_op2.ops[1]); > >> + } > >> + else if (res_op2.code == SSA_NAME > >> + || res_op2.code == INTEGER_CST > >> + || res_op2.code == VECTOR_CST) > >> + { > >> + valueized = true; > >> + return res_op2.ops[0]; > >> + } > >> + } > >> + return valueize_op (op); > >> + }; > >> + auto valueize_name = [&](tree op) > >> + { > >> + return top_valueize ? top_valueize (op) : op; > >> + }; > >> + > >> + int res = gimple_extract (stmt, res_op, valueize_op, valueize_condition, > >> + valueize_name); > >> + if (res == -1) > >> + return false; > >> + > >> + if (res == -2) > >> + { > >> + combined_fn cfn = combined_fn (res_op->code); > >> + if (internal_fn_p (cfn) > >> + && try_conditional_simplification (as_internal_fn (cfn), > >> + res_op, seq, valueize)) > >> + return true; > >> + res = res_op->num_ops; > >> + } > >> + > >> + switch (res) > >> + { > >> + case 0: > >> + return valueized; > >> + case 1: > >> + return (gimple_resimplify1 (seq, res_op, valueize) > >> + || valueized); > >> + case 2: > >> + return (gimple_resimplify2 (seq, res_op, valueize) > >> + || valueized); > >> + case 3: > >> + return (gimple_resimplify3 (seq, res_op, valueize) > >> + || valueized); > >> + case 4: > >> + return (gimple_resimplify4 (seq, res_op, valueize) > >> + || valueized); > >> + case 5: > >> + return (gimple_resimplify5 (seq, res_op, valueize) > >> + || valueized); > > > > can't you use just res.resimplify (seq, valueize) || valueized instead of the > > switch here when res is not zero? > > > > Otherwise looks OK. > > > > Thanks, > > Richard. > > > >> + default: > >> + gcc_unreachable (); > >> + } > >> +} > >> > >> /* Helper for the autogenerated code, valueize OP. */ > >> > >> diff --git a/gcc/gimple-match.h b/gcc/gimple-match.h > >> index 2d4ea476076..15a0f584db7 100644 > >> --- a/gcc/gimple-match.h > >> +++ b/gcc/gimple-match.h > >> @@ -333,6 +333,7 @@ gimple_simplified_result_is_gimple_val (const gimple_match_op *op) > >> > >> extern tree (*mprts_hook) (gimple_match_op *); > >> > >> +bool gimple_extract_op (gimple *, gimple_match_op *); > >> bool gimple_simplify (gimple *, gimple_match_op *, gimple_seq *, > >> tree (*)(tree), tree (*)(tree)); > >> tree maybe_push_res_to_seq (gimple_match_op *, gimple_seq *, > >> -- > >> 2.25.1 > >>