On Wed, Nov 10, 2021 at 1:50 PM Richard Sandiford via Gcc-patches <gcc-patches@gcc.gnu.org> 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. Richard. > 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. > > gcc/testsuite/ > * gcc.dg/fmax-fmin-1.c: New test. > --- > gcc/gimple-fold.c | 25 ++++++++++++++++++++++--- > gcc/testsuite/gcc.dg/fmax-fmin-1.c | 18 ++++++++++++++++++ > 2 files changed, 40 insertions(+), 3 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 a937f130815..6a7d4507c89 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. */ > @@ -6140,18 +6141,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<gcall *> (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/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 >