Re: Ping: [PATCHv3] aarch64/expr: Use ccmp when the outer expression is used twice [PR100942]
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]
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]
> -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]
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; -