From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x12d.google.com (mail-lf1-x12d.google.com [IPv6:2a00:1450:4864:20::12d]) by sourceware.org (Postfix) with ESMTPS id A04B83858C2B for ; Mon, 8 May 2023 06:46:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A04B83858C2B 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-lf1-x12d.google.com with SMTP id 2adb3069b0e04-4f137dbaa4fso4738516e87.2 for ; Sun, 07 May 2023 23:46:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1683528391; x=1686120391; 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=ru+EsOoILzBAYcdL6zvSr3cdYM4O1Yu0F2ffDzhSK8I=; b=BmvbWv+FtPHpeKKahxSC9ZIiBNhEp86SC4gYPHBaOgqy95AqRUovACxGHf1i1sD2MS Miyui5tCouOxi0jnemugcMvHYwJp1HyO2dHNIGvOXrWhghBqitZNfT9UT+l2zr0psvS9 2cogt1Q8ddnNgtVq2HdUxs+mXgbABwKBANv27ABNrBGbisxiBRAxk6fgurutx9JwYslt b720VUokdS7pitQJ05DCrnpqYNqGJ5kyI3OqPdfDL6i6xGRKYGksQKSHTbwuTIwFUJmA GRk84sKD+DKOpiIdB8EBXV7i6Tl1+zsMa7dumgq77WdWPVVvGxxDAC6vUKfLnhQbUHta pF7Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683528391; x=1686120391; 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=ru+EsOoILzBAYcdL6zvSr3cdYM4O1Yu0F2ffDzhSK8I=; b=h14njoBq/KShajNVFs6IihQwE8T615SZBN40G71dMAuzUj6ONIDBhpPLY514Y1COtG hhuVg4T4VsrnalKmZ9LATxfo0a6NAjsIBkKZJ8mpc4dP5xaWe0HriEC2c6Ia8OfSnn56 LyCU74CC2Kottn0djcyXrGNaptu/CWfO0UJJb+cUmjsxxu8lA23EddW0FQogGyzvqXW7 qhT0ph4tXKqK1pJMJGYLY3GNFHZxn8I2MeZ9CE85yk+TjtUHGUt5K0VvvpAiGqRpOp5e MDH/aRzc6GazUdYrVMWR+F+b91vCN1RcV9HqLtK4UKqJSRlFuUPiBOfVo2/5xi3/1dDV 7VyA== X-Gm-Message-State: AC+VfDwFPrm7+bwXyolnhWC3wm4DZHUT8/dS8OUIDlCfSHVQOqttt/Cw C7nmhUNEUPguS+GpzZ4aWpm3I5Gy1txhlvHn1eA= X-Google-Smtp-Source: ACHHUZ6lRvSP7LCcZPJlRD7Gu6V7qtbccSnMJNhpWTYGKjJN+ES3qI3QZmKKw7mpEUU6SNk3LvZSrHgzdf43nTin9TU= X-Received: by 2002:ac2:4846:0:b0:4f0:15dc:4f23 with SMTP id 6-20020ac24846000000b004f015dc4f23mr2238113lfy.29.1683528390772; Sun, 07 May 2023 23:46:30 -0700 (PDT) MIME-Version: 1.0 References: <20230507044332.1122612-1-apinski@marvell.com> <20230507044332.1122612-3-apinski@marvell.com> In-Reply-To: <20230507044332.1122612-3-apinski@marvell.com> From: Richard Biener Date: Mon, 8 May 2023 08:46:18 +0200 Message-ID: Subject: Re: [PATCH 3/3] PHIOPT: factor out unary operations instead of just conversions 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.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,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 Sun, May 7, 2023 at 6:44=E2=80=AFAM Andrew Pinski via Gcc-patches wrote: > > After using factor_out_conditional_conversion with diamond bb, > we should be able do use it also for all normal unary gimple and not > just conversions. This allows to optimize PR 59424 for an example. > This is also a start to optimize PR 64700 and a few others. > > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. OK. > An example of this is: > ``` > static inline unsigned long long g(int t) > { > unsigned t1 =3D t; > return t1; > } > static int abs1(int a) > { > if (a < 0) > a =3D -a; > return a; > } > unsigned long long f(int c, int d, int e) > { > unsigned long long t; > if (d > e) > t =3D g(abs1(d)); > else > t =3D g(abs1(e)); > return t; > } > ``` > > Which should be optimized to: > _9 =3D MAX_EXPR ; > _4 =3D ABS_EXPR <_9>; > t_3 =3D (long long unsigned intD.16) _4; > > gcc/ChangeLog: > > * tree-ssa-phiopt.cc (factor_out_conditional_conversion): Rename = to ... > (factor_out_conditional_operation): This and add support for all = unary > operations. > (pass_phiopt::execute): Update call to factor_out_conditional_con= version > to call factor_out_conditional_operation instead. > > PR tree-optimization/109424 > PR tree-optimization/59424 > > gcc/testsuite/ChangeLog: > > * gcc.dg/tree-ssa/abs-2.c: Update tree scan for > details change in wording. > * gcc.dg/tree-ssa/minmax-17.c: Likewise. > * gcc.dg/tree-ssa/pr103771.c: Likewise. > * gcc.dg/tree-ssa/minmax-18.c: New test. > * gcc.dg/tree-ssa/minmax-19.c: New test. > --- > gcc/testsuite/gcc.dg/tree-ssa/abs-2.c | 2 +- > gcc/testsuite/gcc.dg/tree-ssa/minmax-17.c | 2 +- > gcc/testsuite/gcc.dg/tree-ssa/minmax-18.c | 27 +++++++++++ > gcc/testsuite/gcc.dg/tree-ssa/minmax-19.c | 10 ++++ > gcc/testsuite/gcc.dg/tree-ssa/pr103771.c | 2 +- > gcc/tree-ssa-phiopt.cc | 56 +++++++++++++---------- > 6 files changed, 71 insertions(+), 28 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/minmax-18.c > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/minmax-19.c > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/abs-2.c b/gcc/testsuite/gcc.dg= /tree-ssa/abs-2.c > index 328b1802541..f8bbeb43237 100644 > --- a/gcc/testsuite/gcc.dg/tree-ssa/abs-2.c > +++ b/gcc/testsuite/gcc.dg/tree-ssa/abs-2.c > @@ -16,5 +16,5 @@ test_abs(int *cur) > > /* We should figure out that test_abs has an ABS_EXPR in it. */ > /* { dg-final { scan-tree-dump " =3D ABS_EXPR" "phiopt1"} } */ > -/* { dg-final { scan-tree-dump-times "changed to factor conversion out f= rom" 1 "phiopt1"} } */ > +/* { dg-final { scan-tree-dump-times "changed to factor operation out fr= om" 1 "phiopt1"} } */ > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/minmax-17.c b/gcc/testsuite/gc= c.dg/tree-ssa/minmax-17.c > index bd737e6b4cb..7c76cfc62a9 100644 > --- a/gcc/testsuite/gcc.dg/tree-ssa/minmax-17.c > +++ b/gcc/testsuite/gcc.dg/tree-ssa/minmax-17.c > @@ -18,4 +18,4 @@ unsigned long long test_max(int c, int d, int e) > > /* We should figure out that test_max has an MAX_EXPR in it. */ > /* { dg-final { scan-tree-dump " =3D MAX_EXPR" "phiopt1"} } */ > -/* { dg-final { scan-tree-dump-times "changed to factor conversion out f= rom" 2 "phiopt1"} } */ > +/* { dg-final { scan-tree-dump-times "changed to factor operation out fr= om" 2 "phiopt1"} } */ > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/minmax-18.c b/gcc/testsuite/gc= c.dg/tree-ssa/minmax-18.c > new file mode 100644 > index 00000000000..c8e1670f64a > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/minmax-18.c > @@ -0,0 +1,27 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O -fdump-tree-phiopt1-details" } */ > + > +static inline unsigned long long g(int t) > +{ > + unsigned t1 =3D t; > + return t1; > +} > +static inline int abs1(int a) > +{ > + if (a < 0) > + a =3D -a; > + return a; > +} > +unsigned long long f(int c, int d, int e) > +{ > + unsigned long long t; > + if (d > e) > + t =3D g(abs1(d)); > + else > + t =3D g(abs1(e)); > + return t; > +} > + > +/* { dg-final { scan-tree-dump " =3D MAX_EXPR" "phiopt1"} } */ > +/* { dg-final { scan-tree-dump-times " =3D ABS_EXPR" 2 "phiopt1"} } */ > +/* { dg-final { scan-tree-dump-times "changed to factor operation out fr= om" 3 "phiopt1"} } */ > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/minmax-19.c b/gcc/testsuite/gc= c.dg/tree-ssa/minmax-19.c > new file mode 100644 > index 00000000000..5ed55fe2e23 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/minmax-19.c > @@ -0,0 +1,10 @@ > +/* PR tree-optimization/109424 */ > +/* { dg-do compile } */ > +/* { dg-options "-O -fdump-tree-phiopt1-details" } */ > + > +int f2(int x, int y) > +{ > + return (x > y) ? ~x : ~y; > +} > +/* { dg-final { scan-tree-dump " =3D MAX_EXPR" "phiopt1"} } */ > +/* { dg-final { scan-tree-dump-times "changed to factor operation out fr= om" 1 "phiopt1"} } */ > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr103771.c b/gcc/testsuite/gcc= .dg/tree-ssa/pr103771.c > index 97c9db846cb..8faa45a8222 100644 > --- a/gcc/testsuite/gcc.dg/tree-ssa/pr103771.c > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr103771.c > @@ -1,6 +1,6 @@ > /* { dg-do compile } */ > /* { dg-options "-O3 -fdump-tree-phiopt1-details" } */ > -/* { dg-final { scan-tree-dump-times "changed to factor conversion out f= rom COND_EXPR." 1 "phiopt1" } } */ > +/* { dg-final { scan-tree-dump-times "changed to factor operation out fr= om COND_EXPR." 1 "phiopt1" } } */ > > typedef unsigned char uint8_t; > > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc > index 7fe088b13ff..2fb28b4e60e 100644 > --- a/gcc/tree-ssa-phiopt.cc > +++ b/gcc/tree-ssa-phiopt.cc > @@ -209,13 +209,13 @@ replace_phi_edge_with_variable (basic_block cond_bl= ock, > bb->index); > } > > -/* PR66726: Factor conversion out of COND_EXPR. If the arguments of the= PHI > +/* PR66726: Factor operations out of COND_EXPR. If the arguments of the= PHI > stmt are CONVERT_STMT, factor out the conversion and perform the conv= ersion > to the result of PHI stmt. COND_STMT is the controlling predicate. > Return the newly-created PHI, if any. */ > > static gphi * > -factor_out_conditional_conversion (edge e0, edge e1, gphi *phi, > +factor_out_conditional_operation (edge e0, edge e1, gphi *phi, > tree arg0, tree arg1, gimple *cond_stm= t) > { > gimple *arg0_def_stmt =3D NULL, *arg1_def_stmt =3D NULL, *new_stmt; > @@ -224,7 +224,7 @@ factor_out_conditional_conversion (edge e0, edge e1, = gphi *phi, > gphi *newphi; > gimple_stmt_iterator gsi, gsi_for_def; > location_t locus =3D gimple_location (phi); > - enum tree_code convert_code; > + enum tree_code op_code; > > /* Handle only PHI statements with two arguments. TODO: If all > other arguments to PHI are INTEGER_CST or if their defining > @@ -246,15 +246,17 @@ factor_out_conditional_conversion (edge e0, edge e1= , gphi *phi, > return NULL; > > /* Check if arg0 is an SSA_NAME and the stmt which defines arg0 is > - a conversion. */ > + an unary operation. */ > arg0_def_stmt =3D SSA_NAME_DEF_STMT (arg0); > - if (!gimple_assign_cast_p (arg0_def_stmt)) > + if (!is_gimple_assign (arg0_def_stmt) > + || (gimple_assign_rhs_class (arg0_def_stmt) !=3D GIMPLE_UNARY_RHS > + && gimple_assign_rhs_code (arg0_def_stmt) !=3D VIEW_CONVERT_EXP= R)) > return NULL; > > /* Use the RHS as new_arg0. */ > - convert_code =3D gimple_assign_rhs_code (arg0_def_stmt); > + op_code =3D gimple_assign_rhs_code (arg0_def_stmt); > new_arg0 =3D gimple_assign_rhs1 (arg0_def_stmt); > - if (convert_code =3D=3D VIEW_CONVERT_EXPR) > + if (op_code =3D=3D VIEW_CONVERT_EXPR) > { > new_arg0 =3D TREE_OPERAND (new_arg0, 0); > if (!is_gimple_reg_type (TREE_TYPE (new_arg0))) > @@ -267,10 +269,10 @@ factor_out_conditional_conversion (edge e0, edge e1= , gphi *phi, > if (TREE_CODE (arg1) =3D=3D SSA_NAME) > { > /* Check if arg1 is an SSA_NAME and the stmt which defines arg1 > - is a conversion. */ > + is an unary operation. */ > arg1_def_stmt =3D SSA_NAME_DEF_STMT (arg1); > - if (!is_gimple_assign (arg1_def_stmt) > - || gimple_assign_rhs_code (arg1_def_stmt) !=3D convert_code) > + if (!is_gimple_assign (arg1_def_stmt) > + || gimple_assign_rhs_code (arg1_def_stmt) !=3D op_code) > return NULL; > > /* Either arg1_def_stmt or arg0_def_stmt should be conditional. *= / > @@ -281,7 +283,7 @@ factor_out_conditional_conversion (edge e0, edge e1, = gphi *phi, > > /* Use the RHS as new_arg1. */ > new_arg1 =3D gimple_assign_rhs1 (arg1_def_stmt); > - if (convert_code =3D=3D VIEW_CONVERT_EXPR) > + if (op_code =3D=3D VIEW_CONVERT_EXPR) > new_arg1 =3D TREE_OPERAND (new_arg1, 0); > if (TREE_CODE (new_arg1) =3D=3D SSA_NAME > && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (new_arg1)) > @@ -289,6 +291,10 @@ factor_out_conditional_conversion (edge e0, edge e1,= gphi *phi, > } > else > { > + /* TODO: handle more than just casts here. */ > + if (!gimple_assign_cast_p (arg0_def_stmt)) > + return NULL; > + > /* arg0_def_stmt should be conditional. */ > if (dominated_by_p (CDI_DOMINATORS, gimple_bb (phi), gimple_bb (ar= g0_def_stmt))) > return NULL; > @@ -366,13 +372,13 @@ factor_out_conditional_conversion (edge e0, edge e1= , gphi *phi, > fprintf (dump_file, "PHI "); > print_generic_expr (dump_file, gimple_phi_result (phi)); > fprintf (dump_file, > - " changed to factor conversion out from COND_EXPR.\n"); > - fprintf (dump_file, "New stmt with CAST that defines "); > + " changed to factor operation out from COND_EXPR.\n"); > + fprintf (dump_file, "New stmt with OPERATION that defines "); > print_generic_expr (dump_file, result); > fprintf (dump_file, ".\n"); > } > > - /* Remove the old cast(s) that has single use. */ > + /* Remove the old operation(s) that has single use. */ > gsi_for_def =3D gsi_for_stmt (arg0_def_stmt); > gsi_remove (&gsi_for_def, true); > release_defs (arg0_def_stmt); > @@ -387,14 +393,14 @@ factor_out_conditional_conversion (edge e0, edge e1= , gphi *phi, > add_phi_arg (newphi, new_arg0, e0, locus); > add_phi_arg (newphi, new_arg1, e1, locus); > > - /* Create the conversion stmt and insert it. */ > - if (convert_code =3D=3D VIEW_CONVERT_EXPR) > + /* Create the operation stmt and insert it. */ > + if (op_code =3D=3D VIEW_CONVERT_EXPR) > { > temp =3D fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (result), temp)= ; > new_stmt =3D gimple_build_assign (result, temp); > } > else > - new_stmt =3D gimple_build_assign (result, convert_code, temp); > + new_stmt =3D gimple_build_assign (result, op_code, temp); > gsi =3D gsi_after_labels (gimple_bb (phi)); > gsi_insert_before (&gsi, new_stmt, GSI_SAME_STMT); > > @@ -402,7 +408,7 @@ factor_out_conditional_conversion (edge e0, edge e1, = gphi *phi, > gsi =3D gsi_for_stmt (phi); > gsi_remove (&gsi, true); > > - statistics_counter_event (cfun, "factored out cast", 1); > + statistics_counter_event (cfun, "factored out operation", 1); > > return newphi; > } > @@ -3847,11 +3853,11 @@ gate_hoist_loads (void) > This pass also performs a fifth transformation of a slightly differen= t > flavor. > > - Factor conversion in COND_EXPR > + Factor operations in COND_EXPR > ------------------------------ > > - This transformation factors the conversion out of COND_EXPR with > - factor_out_conditional_conversion. > + This transformation factors the unary operations out of COND_EXPR wit= h > + factor_out_conditional_operation. > > For example: > if (a <=3D CST) goto ; else goto ; > @@ -4092,15 +4098,15 @@ pass_phiopt::execute (function *) > while (newphi) > { > phi =3D newphi; > - /* factor_out_conditional_conversion may create a new PHI i= n > + /* factor_out_conditional_operation may create a new PHI in > BB2 and eliminate an existing PHI in BB2. Recompute val= ues > that may be affected by that change. */ > arg0 =3D gimple_phi_arg_def (phi, e1->dest_idx); > arg1 =3D gimple_phi_arg_def (phi, e2->dest_idx); > gcc_assert (arg0 !=3D NULL_TREE && arg1 !=3D NULL_TREE); > - newphi =3D factor_out_conditional_conversion (e1, e2, phi, > - arg0, arg1, > - cond_stmt); > + newphi =3D factor_out_conditional_operation (e1, e2, phi, > + arg0, arg1, > + cond_stmt); > } > } > > -- > 2.31.1 >