From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x52a.google.com (mail-ed1-x52a.google.com [IPv6:2a00:1450:4864:20::52a]) by sourceware.org (Postfix) with ESMTPS id E2B913858421 for ; Tue, 30 Nov 2021 07:40:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E2B913858421 Received: by mail-ed1-x52a.google.com with SMTP id r25so17569120edq.7 for ; Mon, 29 Nov 2021 23:40:16 -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=KfZwHu1ypFiG+fDfU+MG/jnZJG3SbK2/GENVcveoDfQ=; b=rPZxVrM914yx4GAJC3rnCCqlLEAWL4RU0L3r/NAuug49Kket3aOssfVPxlgzN0i6kB RubnxVOU9WMCQ3NiVOEw7OhQVKF3AkBOEX2k8JjEDDuYYT5oYKoM52fl+JplHnFdVsb+ q+k3RA1KpJmbPoSU8Hhe2mRoK2A5i5NyzVOP1NuUah1Lp+ek1HKYoJ+qrQ+wb/c6xOOJ QuvFPK4VIvg7ZqHFmEKZRj3jKRgfHeh/bcdfCFYt4efVPRjsEB4Q95XwO2XBnTtWHbwQ HpTZOxDFq7o4EoTIU7FVTL5AM25dzP3zHY7QL4xK9wuHkM5XQn0zpmxrjs9PC7Nzeuer GwPQ== X-Gm-Message-State: AOAM530spauYLVYz7EOq53EJrZ/jBBb+CqhS7qpnp8W0SA3mz9rFhDn9 5gU6Hi3u9xj/Pa4dkpFsl4JHD5qcHmrX51x4Dlf9GMZ0 X-Google-Smtp-Source: ABdhPJxT2GMlfJZMPB6hq+9IMg6FbOtE2WGQDu2esryJv4BmCUQsOuHeAow6KTkPG17g92BhHyhVQe0EJMnfr2MZymg= X-Received: by 2002:aa7:c7cf:: with SMTP id o15mr80923432eds.176.1638258015485; Mon, 29 Nov 2021 23:40:15 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Richard Biener Date: Tue, 30 Nov 2021 08:40:04 +0100 Message-ID: Subject: Re: [PATCH 1/4] Canonicalize argument order for commutative functions To: Richard Biener , GCC Patches , Richard Sandiford Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-8.2 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: Tue, 30 Nov 2021 07:40:19 -0000 On Mon, Nov 29, 2021 at 4:40 PM Richard Sandiford wrote: > > Sorry for the slow response, was away last week. > > Richard Biener writes: > > On Wed, Nov 10, 2021 at 1:50 PM Richard Sandiford via Gcc-patches > > wrote: > >> > >> This patch uses information about internal functions to canonicalize > >> the argument order of calls. > >> > >> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? > > > > OK. Note the gimple_resimplifyN functions also canonicalize operand > > order, currently for is_tree_code only: > > > > /* Canonicalize operand order. */ > > bool canonicalized = false; > > if (res_op->code.is_tree_code () > > && (TREE_CODE_CLASS ((enum tree_code) res_op->code) == tcc_comparison > > || commutative_tree_code (res_op->code)) > > && tree_swap_operands_p (res_op->ops[0], res_op->ops[1])) > > { > > std::swap (res_op->ops[0], res_op->ops[1]); > > if (TREE_CODE_CLASS ((enum tree_code) res_op->code) == tcc_comparison) > > res_op->code = swap_tree_comparison (res_op->code); > > canonicalized = true; > > } > > > > that's maybe not the best place. The function assumes the operands > > are already valueized, > > so it maybe should be valueization that does the canonicalization - > > but I think doing it > > elsewhere made operand order unreliable (we do end up with > > non-canonical order in > > the IL sometimes). > > > > So maybe you should amend the code in resimplifyN as well. > > Hmm, yeah, thanks for the heads up. Does this updated version look OK? > Tested as before. Yes - OK. Thanks, Richard. > Thanks, > Richard > > > gcc/ > * gimple-fold.c: Include internal-fn.h. > (fold_stmt_1): If a function maps to an internal one, use > first_commutative_argument to canonicalize the order of > commutative arguments. > * gimple-match-head.c (gimple_resimplify2, gimple_resimplify3) > (gimple_resimplify4, gimple_resimplify5): Extend commutativity > checks to functions. > > gcc/testsuite/ > * gcc.dg/fmax-fmin-1.c: New test. > --- > gcc/gimple-fold.c | 25 ++++++++++++-- > gcc/gimple-match-head.c | 52 ++++++++++++++++++++---------- > gcc/testsuite/gcc.dg/fmax-fmin-1.c | 18 +++++++++++ > 3 files changed, 75 insertions(+), 20 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/fmax-fmin-1.c > > diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c > index 44fba12e150..1d8fd74f72c 100644 > --- a/gcc/gimple-fold.c > +++ b/gcc/gimple-fold.c > @@ -69,6 +69,7 @@ along with GCC; see the file COPYING3. If not see > #include "varasm.h" > #include "memmodel.h" > #include "optabs.h" > +#include "internal-fn.h" > > enum strlen_range_kind { > /* Compute the exact constant string length. */ > @@ -6109,18 +6110,36 @@ fold_stmt_1 (gimple_stmt_iterator *gsi, bool inplace, tree (*valueize) (tree)) > break; > case GIMPLE_CALL: > { > - for (i = 0; i < gimple_call_num_args (stmt); ++i) > + gcall *call = as_a (stmt); > + for (i = 0; i < gimple_call_num_args (call); ++i) > { > - tree *arg = gimple_call_arg_ptr (stmt, i); > + tree *arg = gimple_call_arg_ptr (call, i); > if (REFERENCE_CLASS_P (*arg) > && maybe_canonicalize_mem_ref_addr (arg)) > changed = true; > } > - tree *lhs = gimple_call_lhs_ptr (stmt); > + tree *lhs = gimple_call_lhs_ptr (call); > if (*lhs > && REFERENCE_CLASS_P (*lhs) > && maybe_canonicalize_mem_ref_addr (lhs)) > changed = true; > + if (*lhs) > + { > + combined_fn cfn = gimple_call_combined_fn (call); > + internal_fn ifn = associated_internal_fn (cfn, TREE_TYPE (*lhs)); > + int opno = first_commutative_argument (ifn); > + if (opno >= 0) > + { > + tree arg1 = gimple_call_arg (call, opno); > + tree arg2 = gimple_call_arg (call, opno + 1); > + if (tree_swap_operands_p (arg1, arg2)) > + { > + gimple_call_set_arg (call, opno, arg2); > + gimple_call_set_arg (call, opno + 1, arg1); > + changed = true; > + } > + } > + } > break; > } > case GIMPLE_ASM: > diff --git a/gcc/gimple-match-head.c b/gcc/gimple-match-head.c > index c481a625581..2d9364ca5de 100644 > --- a/gcc/gimple-match-head.c > +++ b/gcc/gimple-match-head.c > @@ -294,18 +294,16 @@ gimple_resimplify2 (gimple_seq *seq, gimple_match_op *res_op, > > /* Canonicalize operand order. */ > bool canonicalized = false; > - if (res_op->code.is_tree_code ()) > + bool is_comparison > + = (res_op->code.is_tree_code () > + && TREE_CODE_CLASS (tree_code (res_op->code)) == tcc_comparison); > + if ((is_comparison || commutative_binary_op_p (res_op->code, res_op->type)) > + && tree_swap_operands_p (res_op->ops[0], res_op->ops[1])) > { > - auto code = tree_code (res_op->code); > - if ((TREE_CODE_CLASS (code) == tcc_comparison > - || commutative_tree_code (code)) > - && tree_swap_operands_p (res_op->ops[0], res_op->ops[1])) > - { > - std::swap (res_op->ops[0], res_op->ops[1]); > - if (TREE_CODE_CLASS (code) == tcc_comparison) > - res_op->code = swap_tree_comparison (code); > - canonicalized = true; > - } > + std::swap (res_op->ops[0], res_op->ops[1]); > + if (is_comparison) > + res_op->code = swap_tree_comparison (tree_code (res_op->code)); > + canonicalized = true; > } > > /* Limit recursion, see gimple_resimplify1. */ > @@ -376,11 +374,11 @@ gimple_resimplify3 (gimple_seq *seq, gimple_match_op *res_op, > > /* Canonicalize operand order. */ > bool canonicalized = false; > - if (res_op->code.is_tree_code () > - && commutative_ternary_tree_code (tree_code (res_op->code)) > - && tree_swap_operands_p (res_op->ops[0], res_op->ops[1])) > + int argno = first_commutative_argument (res_op->code, res_op->type); > + if (argno >= 0 > + && tree_swap_operands_p (res_op->ops[argno], res_op->ops[argno + 1])) > { > - std::swap (res_op->ops[0], res_op->ops[1]); > + std::swap (res_op->ops[argno], res_op->ops[argno + 1]); > canonicalized = true; > } > > @@ -424,6 +422,16 @@ gimple_resimplify4 (gimple_seq *seq, gimple_match_op *res_op, > { > /* No constant folding is defined for four-operand functions. */ > > + /* Canonicalize operand order. */ > + bool canonicalized = false; > + int argno = first_commutative_argument (res_op->code, res_op->type); > + if (argno >= 0 > + && tree_swap_operands_p (res_op->ops[argno], res_op->ops[argno + 1])) > + { > + std::swap (res_op->ops[argno], res_op->ops[argno + 1]); > + canonicalized = true; > + } > + > /* Limit recursion, see gimple_resimplify1. */ > static unsigned depth; > if (depth > 10) > @@ -450,7 +458,7 @@ gimple_resimplify4 (gimple_seq *seq, gimple_match_op *res_op, > if (maybe_resimplify_conditional_op (seq, res_op, valueize)) > return true; > > - return false; > + return canonicalized; > } > > /* Helper that matches and simplifies the toplevel result from > @@ -465,6 +473,16 @@ gimple_resimplify5 (gimple_seq *seq, gimple_match_op *res_op, > { > /* No constant folding is defined for five-operand functions. */ > > + /* Canonicalize operand order. */ > + bool canonicalized = false; > + int argno = first_commutative_argument (res_op->code, res_op->type); > + if (argno >= 0 > + && tree_swap_operands_p (res_op->ops[argno], res_op->ops[argno + 1])) > + { > + std::swap (res_op->ops[argno], res_op->ops[argno + 1]); > + canonicalized = true; > + } > + > gimple_match_op res_op2 (*res_op); > if (gimple_simplify (&res_op2, seq, valueize, > res_op->code, res_op->type, > @@ -478,7 +496,7 @@ gimple_resimplify5 (gimple_seq *seq, gimple_match_op *res_op, > if (maybe_resimplify_conditional_op (seq, res_op, valueize)) > return true; > > - return false; > + return canonicalized; > } > > /* Match and simplify the toplevel valueized operation THIS. > diff --git a/gcc/testsuite/gcc.dg/fmax-fmin-1.c b/gcc/testsuite/gcc.dg/fmax-fmin-1.c > new file mode 100644 > index 00000000000..e7e0518d8bb > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/fmax-fmin-1.c > @@ -0,0 +1,18 @@ > +/* { dg-options "-O -fdump-tree-optimized" } */ > + > +void > +f1 (double *res, double x, double y) > +{ > + res[0] = __builtin_fmax (x, y); > + res[1] = __builtin_fmax (y, x); > +} > + > +void > +f2 (double *res, double x, double y) > +{ > + res[0] = __builtin_fmin (x, y); > + res[1] = __builtin_fmin (y, x); > +} > + > +/* { dg-final { scan-tree-dump-times {__builtin_fmax} 1 "optimized" } } */ > +/* { dg-final { scan-tree-dump-times {__builtin_fmin} 1 "optimized" } } */ > -- > 2.25.1 >