Re: Ping: [PATCHv3] aarch64/expr: Use ccmp when the outer expression is used twice [PR100942]

2024-01-22 Thread Richard Biener
On Mon, Jan 22, 2024 at 2:24 PM Richard Sandiford
 wrote:
>
> Ping for the expr/cfgexpand bits
>
> Richard Sandiford  writes:
> > Andrew Pinski  writes:
> >> Ccmp is not used if the result of the and/ior is used by both
> >> a GIMPLE_COND and a GIMPLE_ASSIGN. This improves the code generation
> >> here by using ccmp in this case.
> >> Two changes is required, first we need to allow the outer statement's
> >> result be used more than once.
> >> The second change is that during the expansion of the gimple, we need
> >> to try using ccmp. This is needed because we don't use expand the ssa
> >> name of the lhs but rather expand directly from the gimple.
> >>
> >> A small note on the ccmp_4.c testcase, we should be able to get slightly
> >> better than with this patch but it is one extra instruction compared to
> >> before.
> >>
> >> Diff from v1:
> >> * v2: Split out expand_gimple_assign_ssa so the we only need to handle
> >> promotion once. Add ccmp_5.c testcase which was suggested. Change comment
> >> on ccmp_candidate_p.
> >
> > I meant more that we should split out the gassign handling in
> > expand_expr_real_1, since we're effectively making cfgexpand follow
> > it more closely.  What do you think about the attached version?
> > Tested on aarch64-linux-gnu and x86_64-linux-gnu.
> >
> > OK for the expr/cfgexpand bits?

OK.

Richard.

> > Thanks,
> > Richard
> >
> > 
> >
> > Ccmp is not used if the result of the and/ior is used by both
> > a GIMPLE_COND and a GIMPLE_ASSIGN. This improves the code generation
> > here by using ccmp in this case.
> > Two changes is required, first we need to allow the outer statement's
> > result be used more than once.
> > The second change is that during the expansion of the gimple, we need
> > to try using ccmp. This is needed because we don't use expand the ssa
> > name of the lhs but rather expand directly from the gimple.
> >
> > A small note on the ccmp_4.c testcase, we should be able to get slightly
> > better than with this patch but it is one extra instruction compared to
> > before.
> >
> >   PR target/100942
> >
> > gcc/ChangeLog:
> >
> >   * ccmp.cc (ccmp_candidate_p): Add outer argument.
> >   Allow if the outer is true and the lhs is used more
> >   than once.
> >   (expand_ccmp_expr): Update call to ccmp_candidate_p.
> >   * expr.h (expand_expr_real_gassign): Declare.
> >   * expr.cc (expand_expr_real_gassign): New function, split out from...
> >   (expand_expr_real_1): ...here.
> >   * cfgexpand.cc (expand_gimple_stmt_1): Use expand_expr_real_gassign.
> >
> > gcc/testsuite/ChangeLog:
> >
> >   * gcc.target/aarch64/ccmp_3.c: New test.
> >   * gcc.target/aarch64/ccmp_4.c: New test.
> >   * gcc.target/aarch64/ccmp_5.c: New test.
> >
> > Signed-off-by: Andrew Pinski 
> > Co-authored-by: Richard Sandiford 
> > ---
> >  gcc/ccmp.cc   |  12 +--
> >  gcc/cfgexpand.cc  |  31 ++-
> >  gcc/expr.cc   | 103 --
> >  gcc/expr.h|   3 +
> >  gcc/testsuite/gcc.target/aarch64/ccmp_3.c |  20 +
> >  gcc/testsuite/gcc.target/aarch64/ccmp_4.c |  35 
> >  gcc/testsuite/gcc.target/aarch64/ccmp_5.c |  20 +
> >  7 files changed, 149 insertions(+), 75 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.target/aarch64/ccmp_3.c
> >  create mode 100644 gcc/testsuite/gcc.target/aarch64/ccmp_4.c
> >  create mode 100644 gcc/testsuite/gcc.target/aarch64/ccmp_5.c
> >
> > diff --git a/gcc/ccmp.cc b/gcc/ccmp.cc
> > index 09d6b5595a4..7cb525addf4 100644
> > --- a/gcc/ccmp.cc
> > +++ b/gcc/ccmp.cc
> > @@ -90,9 +90,10 @@ ccmp_tree_comparison_p (tree t, basic_block bb)
> > If all checks OK in expand_ccmp_expr, it emits insns in prep_seq, then
> > insns in gen_seq.  */
> >
> > -/* Check whether G is a potential conditional compare candidate.  */
> > +/* Check whether G is a potential conditional compare candidate; OUTER is 
> > true if
> > +   G is the outer most AND/IOR.  */
> >  static bool
> > -ccmp_candidate_p (gimple *g)
> > +ccmp_candidate_p (gimple *g, bool outer = false)
> >  {
> >tree lhs, op0, op1;
> >gimple *gs0, *gs1;
> > @@ -109,8 +110,9 @@ ccmp_candidate_p (gimple *g)
> >lhs = gimple_assign_lhs (g);
> >op0 = gimple_assign_rhs1 (g);
> >op1 = gimple_assign_rhs2 (g);
> > -  if ((TREE_CODE (op0) != SSA_NAME) || (TREE_CODE (op1) != SSA_NAME)
> > -  || !has_single_use (lhs))
> > +  if ((TREE_CODE (op0) != SSA_NAME) || (TREE_CODE (op1) != SSA_NAME))
> > +return false;
> > +  if (!outer && !has_single_use (lhs))
> >  return false;
> >
> >bb = gimple_bb (g);
> > @@ -284,7 +286,7 @@ expand_ccmp_expr (gimple *g, machine_mode mode)
> >rtx_insn *last;
> >rtx tmp;
> >
> > -  if (!ccmp_candidate_p (g))
> > +  if (!ccmp_candidate_p (g, true))
> >  return NULL_RTX;
> >
> >last = get_last_insn ();
> > diff --git a/gcc/cfgexpand.cc 

Ping: [PATCHv3] aarch64/expr: Use ccmp when the outer expression is used twice [PR100942]

2024-01-22 Thread Richard Sandiford
Ping for the expr/cfgexpand bits

Richard Sandiford  writes:
> Andrew Pinski  writes:
>> Ccmp is not used if the result of the and/ior is used by both
>> a GIMPLE_COND and a GIMPLE_ASSIGN. This improves the code generation
>> here by using ccmp in this case.
>> Two changes is required, first we need to allow the outer statement's
>> result be used more than once.
>> The second change is that during the expansion of the gimple, we need
>> to try using ccmp. This is needed because we don't use expand the ssa
>> name of the lhs but rather expand directly from the gimple.
>>
>> A small note on the ccmp_4.c testcase, we should be able to get slightly
>> better than with this patch but it is one extra instruction compared to
>> before.
>>
>> Diff from v1:
>> * v2: Split out expand_gimple_assign_ssa so the we only need to handle
>> promotion once. Add ccmp_5.c testcase which was suggested. Change comment
>> on ccmp_candidate_p.
>
> I meant more that we should split out the gassign handling in
> expand_expr_real_1, since we're effectively making cfgexpand follow
> it more closely.  What do you think about the attached version?
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.
>
> OK for the expr/cfgexpand bits?
>
> Thanks,
> Richard
>
> 
>
> Ccmp is not used if the result of the and/ior is used by both
> a GIMPLE_COND and a GIMPLE_ASSIGN. This improves the code generation
> here by using ccmp in this case.
> Two changes is required, first we need to allow the outer statement's
> result be used more than once.
> The second change is that during the expansion of the gimple, we need
> to try using ccmp. This is needed because we don't use expand the ssa
> name of the lhs but rather expand directly from the gimple.
>
> A small note on the ccmp_4.c testcase, we should be able to get slightly
> better than with this patch but it is one extra instruction compared to
> before.
>
>   PR target/100942
>
> gcc/ChangeLog:
>
>   * ccmp.cc (ccmp_candidate_p): Add outer argument.
>   Allow if the outer is true and the lhs is used more
>   than once.
>   (expand_ccmp_expr): Update call to ccmp_candidate_p.
>   * expr.h (expand_expr_real_gassign): Declare.
>   * expr.cc (expand_expr_real_gassign): New function, split out from...
>   (expand_expr_real_1): ...here.
>   * cfgexpand.cc (expand_gimple_stmt_1): Use expand_expr_real_gassign.
>
> gcc/testsuite/ChangeLog:
>
>   * gcc.target/aarch64/ccmp_3.c: New test.
>   * gcc.target/aarch64/ccmp_4.c: New test.
>   * gcc.target/aarch64/ccmp_5.c: New test.
>
> Signed-off-by: Andrew Pinski 
> Co-authored-by: Richard Sandiford 
> ---
>  gcc/ccmp.cc   |  12 +--
>  gcc/cfgexpand.cc  |  31 ++-
>  gcc/expr.cc   | 103 --
>  gcc/expr.h|   3 +
>  gcc/testsuite/gcc.target/aarch64/ccmp_3.c |  20 +
>  gcc/testsuite/gcc.target/aarch64/ccmp_4.c |  35 
>  gcc/testsuite/gcc.target/aarch64/ccmp_5.c |  20 +
>  7 files changed, 149 insertions(+), 75 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/ccmp_3.c
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/ccmp_4.c
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/ccmp_5.c
>
> diff --git a/gcc/ccmp.cc b/gcc/ccmp.cc
> index 09d6b5595a4..7cb525addf4 100644
> --- a/gcc/ccmp.cc
> +++ b/gcc/ccmp.cc
> @@ -90,9 +90,10 @@ ccmp_tree_comparison_p (tree t, basic_block bb)
> If all checks OK in expand_ccmp_expr, it emits insns in prep_seq, then
> insns in gen_seq.  */
>  
> -/* Check whether G is a potential conditional compare candidate.  */
> +/* Check whether G is a potential conditional compare candidate; OUTER is 
> true if
> +   G is the outer most AND/IOR.  */
>  static bool
> -ccmp_candidate_p (gimple *g)
> +ccmp_candidate_p (gimple *g, bool outer = false)
>  {
>tree lhs, op0, op1;
>gimple *gs0, *gs1;
> @@ -109,8 +110,9 @@ ccmp_candidate_p (gimple *g)
>lhs = gimple_assign_lhs (g);
>op0 = gimple_assign_rhs1 (g);
>op1 = gimple_assign_rhs2 (g);
> -  if ((TREE_CODE (op0) != SSA_NAME) || (TREE_CODE (op1) != SSA_NAME)
> -  || !has_single_use (lhs))
> +  if ((TREE_CODE (op0) != SSA_NAME) || (TREE_CODE (op1) != SSA_NAME))
> +return false;
> +  if (!outer && !has_single_use (lhs))
>  return false;
>  
>bb = gimple_bb (g);
> @@ -284,7 +286,7 @@ expand_ccmp_expr (gimple *g, machine_mode mode)
>rtx_insn *last;
>rtx tmp;
>  
> -  if (!ccmp_candidate_p (g))
> +  if (!ccmp_candidate_p (g, true))
>  return NULL_RTX;
>  
>last = get_last_insn ();
> diff --git a/gcc/cfgexpand.cc b/gcc/cfgexpand.cc
> index 1db22f0a1a3..381ed2c82d7 100644
> --- a/gcc/cfgexpand.cc
> +++ b/gcc/cfgexpand.cc
> @@ -3971,37 +3971,18 @@ expand_gimple_stmt_1 (gimple *stmt)
> {
>   rtx target, temp;
>   bool nontemporal = gimple_assign_nontemporal_move_p (assign_stmt);
> - struct 

RE: [PATCHv3] aarch64/expr: Use ccmp when the outer expression is used twice [PR100942]

2024-01-12 Thread Andrew Pinski (QUIC)
> -Original Message-
> From: Richard Sandiford 
> Sent: Friday, January 12, 2024 4:26 AM
> To: Andrew Pinski (QUIC) 
> Cc: gcc-patches@gcc.gnu.org
> Subject: [PATCHv3] aarch64/expr: Use ccmp when the outer expression is
> used twice [PR100942]
> 
> Andrew Pinski  writes:
> > Ccmp is not used if the result of the and/ior is used by both
> > a GIMPLE_COND and a GIMPLE_ASSIGN. This improves the code generation
> > here by using ccmp in this case.
> > Two changes is required, first we need to allow the outer statement's
> > result be used more than once.
> > The second change is that during the expansion of the gimple, we need
> > to try using ccmp. This is needed because we don't use expand the ssa
> > name of the lhs but rather expand directly from the gimple.
> >
> > A small note on the ccmp_4.c testcase, we should be able to get slightly
> > better than with this patch but it is one extra instruction compared to
> > before.
> >
> > Diff from v1:
> > * v2: Split out expand_gimple_assign_ssa so the we only need to handle
> > promotion once. Add ccmp_5.c testcase which was suggested. Change
> comment
> > on ccmp_candidate_p.
> 
> I meant more that we should split out the gassign handling in
> expand_expr_real_1, since we're effectively making cfgexpand follow
> it more closely.  What do you think about the attached version?
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.
> 
> OK for the expr/cfgexpand bits?

Oh that is what I originally thought you wanted but I was not 100% sure so I 
just
moved it out in one place.  Anyways thanks for taking care of the change. 

Thanks,
Andrew Pinski

> 
> Thanks,
> Richard
> 
> 
> 
> Ccmp is not used if the result of the and/ior is used by both
> a GIMPLE_COND and a GIMPLE_ASSIGN. This improves the code generation
> here by using ccmp in this case.
> Two changes is required, first we need to allow the outer statement's
> result be used more than once.
> The second change is that during the expansion of the gimple, we need
> to try using ccmp. This is needed because we don't use expand the ssa
> name of the lhs but rather expand directly from the gimple.
> 
> A small note on the ccmp_4.c testcase, we should be able to get slightly
> better than with this patch but it is one extra instruction compared to
> before.
> 
>   PR target/100942
> 
> gcc/ChangeLog:
> 
>   * ccmp.cc (ccmp_candidate_p): Add outer argument.
>   Allow if the outer is true and the lhs is used more
>   than once.
>   (expand_ccmp_expr): Update call to ccmp_candidate_p.
>   * expr.h (expand_expr_real_gassign): Declare.
>   * expr.cc (expand_expr_real_gassign): New function, split out from...
>   (expand_expr_real_1): ...here.
>   * cfgexpand.cc (expand_gimple_stmt_1): Use
> expand_expr_real_gassign.
> 
> gcc/testsuite/ChangeLog:
> 
>   * gcc.target/aarch64/ccmp_3.c: New test.
>   * gcc.target/aarch64/ccmp_4.c: New test.
>   * gcc.target/aarch64/ccmp_5.c: New test.
> 
> Signed-off-by: Andrew Pinski 
> Co-authored-by: Richard Sandiford 
> ---
>  gcc/ccmp.cc   |  12 +--
>  gcc/cfgexpand.cc  |  31 ++-
>  gcc/expr.cc   | 103 --
>  gcc/expr.h|   3 +
>  gcc/testsuite/gcc.target/aarch64/ccmp_3.c |  20 +
>  gcc/testsuite/gcc.target/aarch64/ccmp_4.c |  35 
>  gcc/testsuite/gcc.target/aarch64/ccmp_5.c |  20 +
>  7 files changed, 149 insertions(+), 75 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/ccmp_3.c
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/ccmp_4.c
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/ccmp_5.c
> 
> diff --git a/gcc/ccmp.cc b/gcc/ccmp.cc
> index 09d6b5595a4..7cb525addf4 100644
> --- a/gcc/ccmp.cc
> +++ b/gcc/ccmp.cc
> @@ -90,9 +90,10 @@ ccmp_tree_comparison_p (tree t, basic_block bb)
> If all checks OK in expand_ccmp_expr, it emits insns in prep_seq, then
> insns in gen_seq.  */
> 
> -/* Check whether G is a potential conditional compare candidate.  */
> +/* Check whether G is a potential conditional compare candidate; OUTER is
> true if
> +   G is the outer most AND/IOR.  */
>  static bool
> -ccmp_candidate_p (gimple *g)
> +ccmp_candidate_p (gimple *g, bool outer = false)
>  {
>tree lhs, op0, op1;
>gimple *gs0, *gs1;
> @@ -109,8 +110,9 @@ ccmp_candidate_p (gimple *g)
>lhs = gimple_assign_lhs (g);
>op0 = gimple_assign_rhs1 (g);
>op1 = gimple_assign_rhs2 (g);
> -  if ((TREE_CODE (op0) != SSA_NAME) || (TREE_CODE (op1) != SSA_NAME)
> -  |

[PATCHv3] aarch64/expr: Use ccmp when the outer expression is used twice [PR100942]

2024-01-12 Thread Richard Sandiford
Andrew Pinski  writes:
> Ccmp is not used if the result of the and/ior is used by both
> a GIMPLE_COND and a GIMPLE_ASSIGN. This improves the code generation
> here by using ccmp in this case.
> Two changes is required, first we need to allow the outer statement's
> result be used more than once.
> The second change is that during the expansion of the gimple, we need
> to try using ccmp. This is needed because we don't use expand the ssa
> name of the lhs but rather expand directly from the gimple.
>
> A small note on the ccmp_4.c testcase, we should be able to get slightly
> better than with this patch but it is one extra instruction compared to
> before.
>
> Diff from v1:
> * v2: Split out expand_gimple_assign_ssa so the we only need to handle
> promotion once. Add ccmp_5.c testcase which was suggested. Change comment
> on ccmp_candidate_p.

I meant more that we should split out the gassign handling in
expand_expr_real_1, since we're effectively making cfgexpand follow
it more closely.  What do you think about the attached version?
Tested on aarch64-linux-gnu and x86_64-linux-gnu.

OK for the expr/cfgexpand bits?

Thanks,
Richard



Ccmp is not used if the result of the and/ior is used by both
a GIMPLE_COND and a GIMPLE_ASSIGN. This improves the code generation
here by using ccmp in this case.
Two changes is required, first we need to allow the outer statement's
result be used more than once.
The second change is that during the expansion of the gimple, we need
to try using ccmp. This is needed because we don't use expand the ssa
name of the lhs but rather expand directly from the gimple.

A small note on the ccmp_4.c testcase, we should be able to get slightly
better than with this patch but it is one extra instruction compared to
before.

PR target/100942

gcc/ChangeLog:

* ccmp.cc (ccmp_candidate_p): Add outer argument.
Allow if the outer is true and the lhs is used more
than once.
(expand_ccmp_expr): Update call to ccmp_candidate_p.
* expr.h (expand_expr_real_gassign): Declare.
* expr.cc (expand_expr_real_gassign): New function, split out from...
(expand_expr_real_1): ...here.
* cfgexpand.cc (expand_gimple_stmt_1): Use expand_expr_real_gassign.

gcc/testsuite/ChangeLog:

* gcc.target/aarch64/ccmp_3.c: New test.
* gcc.target/aarch64/ccmp_4.c: New test.
* gcc.target/aarch64/ccmp_5.c: New test.

Signed-off-by: Andrew Pinski 
Co-authored-by: Richard Sandiford 
---
 gcc/ccmp.cc   |  12 +--
 gcc/cfgexpand.cc  |  31 ++-
 gcc/expr.cc   | 103 --
 gcc/expr.h|   3 +
 gcc/testsuite/gcc.target/aarch64/ccmp_3.c |  20 +
 gcc/testsuite/gcc.target/aarch64/ccmp_4.c |  35 
 gcc/testsuite/gcc.target/aarch64/ccmp_5.c |  20 +
 7 files changed, 149 insertions(+), 75 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/ccmp_3.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/ccmp_4.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/ccmp_5.c

diff --git a/gcc/ccmp.cc b/gcc/ccmp.cc
index 09d6b5595a4..7cb525addf4 100644
--- a/gcc/ccmp.cc
+++ b/gcc/ccmp.cc
@@ -90,9 +90,10 @@ ccmp_tree_comparison_p (tree t, basic_block bb)
If all checks OK in expand_ccmp_expr, it emits insns in prep_seq, then
insns in gen_seq.  */
 
-/* Check whether G is a potential conditional compare candidate.  */
+/* Check whether G is a potential conditional compare candidate; OUTER is true 
if
+   G is the outer most AND/IOR.  */
 static bool
-ccmp_candidate_p (gimple *g)
+ccmp_candidate_p (gimple *g, bool outer = false)
 {
   tree lhs, op0, op1;
   gimple *gs0, *gs1;
@@ -109,8 +110,9 @@ ccmp_candidate_p (gimple *g)
   lhs = gimple_assign_lhs (g);
   op0 = gimple_assign_rhs1 (g);
   op1 = gimple_assign_rhs2 (g);
-  if ((TREE_CODE (op0) != SSA_NAME) || (TREE_CODE (op1) != SSA_NAME)
-  || !has_single_use (lhs))
+  if ((TREE_CODE (op0) != SSA_NAME) || (TREE_CODE (op1) != SSA_NAME))
+return false;
+  if (!outer && !has_single_use (lhs))
 return false;
 
   bb = gimple_bb (g);
@@ -284,7 +286,7 @@ expand_ccmp_expr (gimple *g, machine_mode mode)
   rtx_insn *last;
   rtx tmp;
 
-  if (!ccmp_candidate_p (g))
+  if (!ccmp_candidate_p (g, true))
 return NULL_RTX;
 
   last = get_last_insn ();
diff --git a/gcc/cfgexpand.cc b/gcc/cfgexpand.cc
index 1db22f0a1a3..381ed2c82d7 100644
--- a/gcc/cfgexpand.cc
+++ b/gcc/cfgexpand.cc
@@ -3971,37 +3971,18 @@ expand_gimple_stmt_1 (gimple *stmt)
  {
rtx target, temp;
bool nontemporal = gimple_assign_nontemporal_move_p (assign_stmt);
-   struct separate_ops ops;
bool promoted = false;
 
target = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE);
if (GET_CODE (target) == SUBREG && SUBREG_PROMOTED_VAR_P (target))
  promoted = true;
 
-