| Message ID | 20231128023227.36200-2-gaofei@eswincomputing.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/4,RISC-V] prefer Zicond primitive semantics to SFB | expand |
On 11/27/23 19:32, Fei Gao wrote: > op=[PLUS, MINUS, IOR, XOR, ASHIFT, ASHIFTRT, LSHIFTRT, ROTATE, ROTATERT] > > SIGN_EXTEND, ZERO_EXTEND and SUBREG has been considered > to support SImode in 64-bit machine. Let's defer these for now. We're supposed to be wrapping up work that was posted before stage1 closed. If these opcodes were trivial to support, then I would let them through, but SUBREGs for example can be problematical as their semantics can be complex. > > Conditional op, if zero > rd = (rc == 0) ? (rs1 op rs2) : rs1 > --> > czero.nez rd, rs2, rc > op rd, rs1, rd > > Conditional op, if non-zero > rd = (rc != 0) ? (rs1 op rs2) : rs1 > --> > czero.eqz rd, rs2, rc > op rd, rs1, rd > > Co-authored-by: Xiao Zeng<zengxiao@eswincomputing.com> > > gcc/ChangeLog: > > * ifcvt.cc (noce_try_cond_zero_arith):handler for condtional zero based ifcvt > (noce_emit_czero): helper for noce_try_cond_zero_arith > (noce_cond_zero_binary_op_supported): check supported OPs for condtional zero based ifcvt > (get_base_reg): get base reg of a subreg or the reg itself > (noce_bbs_ok_for_cond_zero_arith): check if BBs are OK for condtional zero based ifcvt > (noce_process_if_block): add noce_try_cond_zero_arith > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/zicond_ifcvt_opt.c: New test. > --- > gcc/ifcvt.cc | 210 ++++++ > .../gcc.target/riscv/zicond_ifcvt_opt.c | 682 ++++++++++++++++++ > 2 files changed, 892 insertions(+) > create mode 100644 gcc/testsuite/gcc.target/riscv/zicond_ifcvt_opt.c > > diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc > index a0af553b9ff..8f6a0e7f5fe 100644 > --- a/gcc/ifcvt.cc > +++ b/gcc/ifcvt.cc > @@ -787,6 +787,7 @@ static rtx noce_get_alt_condition (struct noce_if_info *, rtx, rtx_insn **); > static bool noce_try_minmax (struct noce_if_info *); > static bool noce_try_abs (struct noce_if_info *); > static bool noce_try_sign_mask (struct noce_if_info *); > +static int noce_try_cond_zero_arith (struct noce_if_info *); > > /* Return the comparison code for reversed condition for IF_INFO, > or UNKNOWN if reversing the condition is not possible. */ > @@ -1831,6 +1832,40 @@ noce_emit_cmove (struct noce_if_info *if_info, rtx x, enum rtx_code code, > return NULL_RTX; > } > > +/* Emit a conditional zero, returning TARGET or NULL_RTX upon failure. > + IF_INFO describes the if-conversion scenario under consideration. > + CZERO_CODE selects the condition (EQ/NE). > + NON_ZERO_OP is the nonzero operand of the conditional move > + TARGET is the desired output register. */ > + > +static rtx > +noce_emit_czero (struct noce_if_info *if_info, enum rtx_code czero_code, > + rtx non_zero_op, rtx target) [ ... ] The code you wrote is safe in that if constructs a suitable if-then-else as a single object, starts a new sequence the uses emit_insn to put that object onto a sequence. Then you extract that one and only one insn from the sequence and validate it can be recognized. In cases where you want to do something like this and know you're going to emit one and only one insn you can use 'make_insn_raw' without generating a new sequence. I would suggest you replace all the code starting with start_sequence() and ending with end_sequence () (inclusive) with something like insn = make_insn_raw (set); if (recog_memoized (insn) >= 0) { emit_insn (insn); return target; } return NULL_RTX; Note that in the future (gcc-15) when this code is generalized to use the expander it will potentially generate multiple insns at which point we'll have to put them on a sequence and validate they all are recognizable. But we'll tackle that at the appropriate time. > + > + return false; > +} > + > +/* Helper function to return REG itself or inner expression of a SUBREG, > + otherwise NULL_RTX for other RTX_CODE. */ > + > +static rtx > +get_base_reg (rtx exp) > +{ > + if (REG_P (exp)) > + return exp; > + else if (SUBREG_P (exp)) > + return SUBREG_REG (exp); > + > + return NULL_RTX; > + I would advise against handling subregs at this point. What you're doing is probably too simplistic given the semantics of subregs. > + > + /* Strip sign_extend if any. */ > + if (GET_CODE (a) == SIGN_EXTEND || GET_CODE (a) == ZERO_EXTEND) > + bin_exp = XEXP (a, 0); > + else > + bin_exp = a; Similarly while I do think we're going to want to handle extensions, let's not try and add them at this point. We want to get this wrapped up & integrated so that everyone can move their focus to bugfixing for gcc-14. > + > +/* Try to covert if-then-else with conditional zero, > + returning TURE on success or FALSE on failure. > + IF_INFO describes the if-conversion scenario under consideration. */ > + > +static int > +noce_try_cond_zero_arith (struct noce_if_info *if_info) > +{ > + rtx target, a; > + rtx_insn *seq; > + machine_mode mode = GET_MODE (if_info->x); > + rtx common = NULL_RTX; > + enum rtx_code czero_code = UNKNOWN; > + rtx non_zero_op = NULL_RTX; > + rtx *to_replace = NULL; > + > + if (!noce_bbs_ok_for_cond_zero_arith (if_info, &common, &czero_code, &a, > + &to_replace)) > + return false; > + > + /* Disallow CONST_INT currently for simplicity*/ > + if (to_replace == NULL || !REG_P (*to_replace)) > + return false;If you're not going to allow CONST_INT reject them in the ok_for_condzero_arith routine. Presumably you're rejecting constants because zicond doesn't allow them in the arms. We'll want to handle them in the future and can do so easily once we're using the expander. > + > + non_zero_op = *to_replace; > + > + start_sequence (); > + > + /* If x is used in both input and out like x = c ? x + z : x, > + use a new reg to avoid modifying x */ > + if (common && rtx_equal_p (common, if_info->x)) > + target = gen_reg_rtx (mode); > + else > + target = if_info->x; > + > + target = noce_emit_czero (if_info, czero_code, non_zero_op, target); > + if (!target || !to_replace) > + { > + end_sequence (); > + return false; > + } > + > + *to_replace = target; > + emit_insn (gen_rtx_SET (if_info->x, a)); This isn't necessarily safe. Use emit_move_insn instead. This is pretty close. Hopefully one more iteration and it can be integrated. jeff
On 2023-11-29 13:26 Jeff Law <jeffreyalaw@gmail.com> wrote: > > > >On 11/27/23 19:32, Fei Gao wrote: >> op=[PLUS, MINUS, IOR, XOR, ASHIFT, ASHIFTRT, LSHIFTRT, ROTATE, ROTATERT] >> >> SIGN_EXTEND, ZERO_EXTEND and SUBREG has been considered >> to support SImode in 64-bit machine. >Let's defer these for now. We're supposed to be wrapping up work that >was posted before stage1 closed. If these opcodes were trivial to >support, then I would let them through, but SUBREGs for example can be >problematical as their semantics can be complex. I can delete the 32bit-support in 64 bit machine. Or split this patch into 2 parts word-size support and 32bit-support in 64 bit machine. Or keep current status if the following words persuades you :) Regarding complexity, the current patch differs from 1st version by "to_replace" and reduces complexity significantly. noce_simple_bbs() ensures we have only single set for insn_a like x=sext(subreg(y) op subreg(z)). Instead of constructing an insn considering complex extension and subreg from scratch, to_replace locates the rtx z in noce_bbs_ok_for_cond_zero_arith, and then replace it with czero result. In this way, extension and subreg are as simple as reg cases. All the cases for extension and subreg have been uploaded in this patch. They can be found by searching "int" in gcc.target/riscv/zicond_ifcvt_opt.c Could you please let me known which you prefer? > > >> >> Conditional op, if zero >> rd = (rc == 0) ? (rs1 op rs2) : rs1 >> --> >> czero.nez rd, rs2, rc >> op rd, rs1, rd >> >> Conditional op, if non-zero >> rd = (rc != 0) ? (rs1 op rs2) : rs1 >> --> >> czero.eqz rd, rs2, rc >> op rd, rs1, rd >> >> Co-authored-by: Xiao Zeng<zengxiao@eswincomputing.com> >> >> gcc/ChangeLog: >> >> * ifcvt.cc (noce_try_cond_zero_arith):handler for condtional zero based ifcvt >> (noce_emit_czero): helper for noce_try_cond_zero_arith >> (noce_cond_zero_binary_op_supported): check supported OPs for condtional zero based ifcvt >> (get_base_reg): get base reg of a subreg or the reg itself >> (noce_bbs_ok_for_cond_zero_arith): check if BBs are OK for condtional zero based ifcvt >> (noce_process_if_block): add noce_try_cond_zero_arith >> >> gcc/testsuite/ChangeLog: >> >> * gcc.target/riscv/zicond_ifcvt_opt.c: New test. >> --- >> gcc/ifcvt.cc | 210 ++++++ >> .../gcc.target/riscv/zicond_ifcvt_opt.c | 682 ++++++++++++++++++ >> 2 files changed, 892 insertions(+) >> create mode 100644 gcc/testsuite/gcc.target/riscv/zicond_ifcvt_opt.c >> >> diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc >> index a0af553b9ff..8f6a0e7f5fe 100644 >> --- a/gcc/ifcvt.cc >> +++ b/gcc/ifcvt.cc >> @@ -787,6 +787,7 @@ static rtx noce_get_alt_condition (struct noce_if_info *, rtx, rtx_insn **); >> static bool noce_try_minmax (struct noce_if_info *); >> static bool noce_try_abs (struct noce_if_info *); >> static bool noce_try_sign_mask (struct noce_if_info *); >> +static int noce_try_cond_zero_arith (struct noce_if_info *); >> >> /* Return the comparison code for reversed condition for IF_INFO, >> or UNKNOWN if reversing the condition is not possible. */ >> @@ -1831,6 +1832,40 @@ noce_emit_cmove (struct noce_if_info *if_info, rtx x, enum rtx_code code, >> return NULL_RTX; >> } >> >> +/* Emit a conditional zero, returning TARGET or NULL_RTX upon failure. >> + IF_INFO describes the if-conversion scenario under consideration. >> + CZERO_CODE selects the condition (EQ/NE). >> + NON_ZERO_OP is the nonzero operand of the conditional move >> + TARGET is the desired output register. */ >> + >> +static rtx >> +noce_emit_czero (struct noce_if_info *if_info, enum rtx_code czero_code, >> + rtx non_zero_op, rtx target) >[ ... ] >The code you wrote is safe in that if constructs a suitable if-then-else >as a single object, starts a new sequence the uses emit_insn to put that >object onto a sequence. Then you extract that one and only one insn >from the sequence and validate it can be recognized. > >In cases where you want to do something like this and know you're going >to emit one and only one insn you can use 'make_insn_raw' without >generating a new sequence. > >I would suggest you replace all the code starting with start_sequence() >and ending with end_sequence () (inclusive) with something like > >insn = make_insn_raw (set); >if (recog_memoized (insn) >= 0) > { > emit_insn (insn); > return target; > } >return NULL_RTX; Thanks for your advice, I will take it in next version. > > >Note that in the future (gcc-15) when this code is generalized to use >the expander it will potentially generate multiple insns at which point >we'll have to put them on a sequence and validate they all are >recognizable. But we'll tackle that at the appropriate time. Sure. > > >> + >> + return false; >> +} >> + >> +/* Helper function to return REG itself or inner expression of a SUBREG, >> + otherwise NULL_RTX for other RTX_CODE. */ >> + >> +static rtx >> +get_base_reg (rtx exp) >> +{ >> + if (REG_P (exp)) >> + return exp; >> + else if (SUBREG_P (exp)) >> + return SUBREG_REG (exp); >> + >> + return NULL_RTX; >> + >I would advise against handling subregs at this point. What you're >doing is probably too simplistic given the semantics of subregs. > > > >> + >> + /* Strip sign_extend if any. */ >> + if (GET_CODE (a) == SIGN_EXTEND || GET_CODE (a) == ZERO_EXTEND) >> + bin_exp = XEXP (a, 0); >> + else >> + bin_exp = a; >Similarly while I do think we're going to want to handle extensions, >let's not try and add them at this point. We want to get this wrapped >up & integrated so that everyone can move their focus to bugfixing for >gcc-14. Same reply with the one regarding *SIGN_EXTEND, ZERO_EXTEND and SUBREG* at the beginning. > >> + >> +/* Try to covert if-then-else with conditional zero, >> + returning TURE on success or FALSE on failure. >> + IF_INFO describes the if-conversion scenario under consideration. */ >> + >> +static int >> +noce_try_cond_zero_arith (struct noce_if_info *if_info) >> +{ >> + rtx target, a; >> + rtx_insn *seq; >> + machine_mode mode = GET_MODE (if_info->x); >> + rtx common = NULL_RTX; >> + enum rtx_code czero_code = UNKNOWN; >> + rtx non_zero_op = NULL_RTX; >> + rtx *to_replace = NULL; >> + >> + if (!noce_bbs_ok_for_cond_zero_arith (if_info, &common, &czero_code, &a, >> + &to_replace)) >> + return false; >> + >> + /* Disallow CONST_INT currently for simplicity*/ >> + if (to_replace == NULL || !REG_P (*to_replace)) >> + return false;If you're not going to allow CONST_INT reject them in the >ok_for_condzero_arith routine. Presumably you're rejecting constants >because zicond doesn't allow them in the arms. We'll want to handle >them in the future and can do so easily once we're using the expander. I remembered the shift-by-6 case mentioned by you, see [PATCH 3/4] for const_int support:) https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg327149.html Also AND is supported in https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg327219.html Could you please reveiw 2 patches above so I will repost together if needed? > > > >> + >> + non_zero_op = *to_replace; >> + >> + start_sequence (); >> + >> + /* If x is used in both input and out like x = c ? x + z : x, >> + use a new reg to avoid modifying x */ >> + if (common && rtx_equal_p (common, if_info->x)) >> + target = gen_reg_rtx (mode); >> + else >> + target = if_info->x; >> + >> + target = noce_emit_czero (if_info, czero_code, non_zero_op, target); >> + if (!target || !to_replace) >> + { >> + end_sequence (); >> + return false; >> + } >> + >> + *to_replace = target; >> + emit_insn (gen_rtx_SET (if_info->x, a)); >This isn't necessarily safe. Use emit_move_insn instead. OK. Thanks & BR Fei > >This is pretty close. Hopefully one more iteration and it can be >integrated. > >jeff
On 2023-11-29 19:09 Fei Gao <gaofei@eswincomputing.com> wrote: > >On 2023-11-29 13:26 Jeff Law <jeffreyalaw@gmail.com> wrote: >> >> >> >>On 11/27/23 19:32, Fei Gao wrote: >>> op=[PLUS, MINUS, IOR, XOR, ASHIFT, ASHIFTRT, LSHIFTRT, ROTATE, ROTATERT] >>> >>> SIGN_EXTEND, ZERO_EXTEND and SUBREG has been considered >>> to support SImode in 64-bit machine. >>Let's defer these for now. We're supposed to be wrapping up work that >>was posted before stage1 closed. If these opcodes were trivial to >>support, then I would let them through, but SUBREGs for example can be >>problematical as their semantics can be complex. > >I can delete the 32bit-support in 64 bit machine. >Or split this patch into 2 parts word-size support and 32bit-support in 64 bit machine. >Or keep current status if the following words persuades you :) > >Regarding complexity, the current patch differs from 1st version by "to_replace" and reduces >complexity significantly. noce_simple_bbs() ensures we have only single set for insn_a like x=sext(subreg(y) op subreg(z)). >Instead of constructing an insn considering complex extension and subreg from scratch, to_replace locates the rtx z >in noce_bbs_ok_for_cond_zero_arith, >and then replace it with czero result. In this way, extension and subreg are as simple as reg cases. >All the cases for extension and subreg have been uploaded in this patch. >They can be found by searching "int" in gcc.target/riscv/zicond_ifcvt_opt.c > >Could you please let me known which you prefer? I split the patches. See new series https://patchwork.sourceware.org/project/gcc/list/?series=27924 > >> >> >>> >>> Conditional op, if zero >>> rd = (rc == 0) ? (rs1 op rs2) : rs1 >>> --> >>> czero.nez rd, rs2, rc >>> op rd, rs1, rd >>> >>> Conditional op, if non-zero >>> rd = (rc != 0) ? (rs1 op rs2) : rs1 >>> --> >>> czero.eqz rd, rs2, rc >>> op rd, rs1, rd >>> >>> Co-authored-by: Xiao Zeng<zengxiao@eswincomputing.com> >>> >>> gcc/ChangeLog: >>> >>> * ifcvt.cc (noce_try_cond_zero_arith):handler for condtional zero based ifcvt >>> (noce_emit_czero): helper for noce_try_cond_zero_arith >>> (noce_cond_zero_binary_op_supported): check supported OPs for condtional zero based ifcvt >>> (get_base_reg): get base reg of a subreg or the reg itself >>> (noce_bbs_ok_for_cond_zero_arith): check if BBs are OK for condtional zero based ifcvt >>> (noce_process_if_block): add noce_try_cond_zero_arith >>> >>> gcc/testsuite/ChangeLog: >>> >>> * gcc.target/riscv/zicond_ifcvt_opt.c: New test. >>> --- >>> gcc/ifcvt.cc | 210 ++++++ >>> .../gcc.target/riscv/zicond_ifcvt_opt.c | 682 ++++++++++++++++++ >>> 2 files changed, 892 insertions(+) >>> create mode 100644 gcc/testsuite/gcc.target/riscv/zicond_ifcvt_opt.c >>> >>> diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc >>> index a0af553b9ff..8f6a0e7f5fe 100644 >>> --- a/gcc/ifcvt.cc >>> +++ b/gcc/ifcvt.cc >>> @@ -787,6 +787,7 @@ static rtx noce_get_alt_condition (struct noce_if_info *, rtx, rtx_insn **); >>> static bool noce_try_minmax (struct noce_if_info *); >>> static bool noce_try_abs (struct noce_if_info *); >>> static bool noce_try_sign_mask (struct noce_if_info *); >>> +static int noce_try_cond_zero_arith (struct noce_if_info *); >>> >>> /* Return the comparison code for reversed condition for IF_INFO, >>> or UNKNOWN if reversing the condition is not possible. */ >>> @@ -1831,6 +1832,40 @@ noce_emit_cmove (struct noce_if_info *if_info, rtx x, enum rtx_code code, >>> return NULL_RTX; >>> } >>> >>> +/* Emit a conditional zero, returning TARGET or NULL_RTX upon failure. >>> + IF_INFO describes the if-conversion scenario under consideration. >>> + CZERO_CODE selects the condition (EQ/NE). >>> + NON_ZERO_OP is the nonzero operand of the conditional move >>> + TARGET is the desired output register. */ >>> + >>> +static rtx >>> +noce_emit_czero (struct noce_if_info *if_info, enum rtx_code czero_code, >>> + rtx non_zero_op, rtx target) >>[ ... ] >>The code you wrote is safe in that if constructs a suitable if-then-else >>as a single object, starts a new sequence the uses emit_insn to put that >>object onto a sequence. Then you extract that one and only one insn >>from the sequence and validate it can be recognized. >> >>In cases where you want to do something like this and know you're going >>to emit one and only one insn you can use 'make_insn_raw' without >>generating a new sequence. >> >>I would suggest you replace all the code starting with start_sequence() >>and ending with end_sequence () (inclusive) with something like >> >>insn = make_insn_raw (set); >>if (recog_memoized (insn) >= 0) >> { >> emit_insn (insn); >> return target; >> } >>return NULL_RTX; >Thanks for your advice, I will take it in next version. I used add_insn instead of emit_insn to avoid a segment fault caused by invalid NEXT_INSN (insn) of the raw insn. BR, Fei > >> >> >>Note that in the future (gcc-15) when this code is generalized to use >>the expander it will potentially generate multiple insns at which point >>we'll have to put them on a sequence and validate they all are >>recognizable. But we'll tackle that at the appropriate time. >Sure. > >> >> >>> + >>> + return false; >>> +} >>> + >>> +/* Helper function to return REG itself or inner expression of a SUBREG, >>> + otherwise NULL_RTX for other RTX_CODE. */ >>> + >>> +static rtx >>> +get_base_reg (rtx exp) >>> +{ >>> + if (REG_P (exp)) >>> + return exp; >>> + else if (SUBREG_P (exp)) >>> + return SUBREG_REG (exp); >>> + >>> + return NULL_RTX; >>> + >>I would advise against handling subregs at this point. What you're >>doing is probably too simplistic given the semantics of subregs. >> >> >> >>> + >>> + /* Strip sign_extend if any. */ >>> + if (GET_CODE (a) == SIGN_EXTEND || GET_CODE (a) == ZERO_EXTEND) >>> + bin_exp = XEXP (a, 0); >>> + else >>> + bin_exp = a; >>Similarly while I do think we're going to want to handle extensions, >>let's not try and add them at this point. We want to get this wrapped >>up & integrated so that everyone can move their focus to bugfixing for >>gcc-14. >Same reply with the one regarding *SIGN_EXTEND, ZERO_EXTEND and SUBREG* at >the beginning. > >> >>> + >>> +/* Try to covert if-then-else with conditional zero, >>> + returning TURE on success or FALSE on failure. >>> + IF_INFO describes the if-conversion scenario under consideration. */ >>> + >>> +static int >>> +noce_try_cond_zero_arith (struct noce_if_info *if_info) >>> +{ >>> + rtx target, a; >>> + rtx_insn *seq; >>> + machine_mode mode = GET_MODE (if_info->x); >>> + rtx common = NULL_RTX; >>> + enum rtx_code czero_code = UNKNOWN; >>> + rtx non_zero_op = NULL_RTX; >>> + rtx *to_replace = NULL; >>> + >>> + if (!noce_bbs_ok_for_cond_zero_arith (if_info, &common, &czero_code, &a, >>> + &to_replace)) >>> + return false; >>> + >>> + /* Disallow CONST_INT currently for simplicity*/ >>> + if (to_replace == NULL || !REG_P (*to_replace)) >>> + return false;If you're not going to allow CONST_INT reject them in the >>ok_for_condzero_arith routine. Presumably you're rejecting constants >>because zicond doesn't allow them in the arms. We'll want to handle >>them in the future and can do so easily once we're using the expander. > >I remembered the shift-by-6 case mentioned by you, see [PATCH 3/4] for const_int support:) >https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg327149.html > >Also AND is supported in >https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg327219.html > >Could you please reveiw 2 patches above so I will repost together if needed? >> >> >> >>> + >>> + non_zero_op = *to_replace; >>> + >>> + start_sequence (); >>> + >>> + /* If x is used in both input and out like x = c ? x + z : x, >>> + use a new reg to avoid modifying x */ >>> + if (common && rtx_equal_p (common, if_info->x)) >>> + target = gen_reg_rtx (mode); >>> + else >>> + target = if_info->x; >>> + >>> + target = noce_emit_czero (if_info, czero_code, non_zero_op, target); >>> + if (!target || !to_replace) >>> + { >>> + end_sequence (); >>> + return false; >>> + } >>> + >>> + *to_replace = target; >>> + emit_insn (gen_rtx_SET (if_info->x, a)); >>This isn't necessarily safe. Use emit_move_insn instead. >OK. > >Thanks & BR >Fei > >> >>This is pretty close. Hopefully one more iteration and it can be >>integrated. >> >>jeff
diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc index a0af553b9ff..8f6a0e7f5fe 100644 --- a/gcc/ifcvt.cc +++ b/gcc/ifcvt.cc @@ -787,6 +787,7 @@ static rtx noce_get_alt_condition (struct noce_if_info *, rtx, rtx_insn **); static bool noce_try_minmax (struct noce_if_info *); static bool noce_try_abs (struct noce_if_info *); static bool noce_try_sign_mask (struct noce_if_info *); +static int noce_try_cond_zero_arith (struct noce_if_info *); /* Return the comparison code for reversed condition for IF_INFO, or UNKNOWN if reversing the condition is not possible. */ @@ -1831,6 +1832,40 @@ noce_emit_cmove (struct noce_if_info *if_info, rtx x, enum rtx_code code, return NULL_RTX; } +/* Emit a conditional zero, returning TARGET or NULL_RTX upon failure. + IF_INFO describes the if-conversion scenario under consideration. + CZERO_CODE selects the condition (EQ/NE). + NON_ZERO_OP is the nonzero operand of the conditional move + TARGET is the desired output register. */ + +static rtx +noce_emit_czero (struct noce_if_info *if_info, enum rtx_code czero_code, + rtx non_zero_op, rtx target) +{ + machine_mode mode = GET_MODE (target); + rtx cond_op0 = XEXP (if_info->cond, 0); + rtx czero_cond + = gen_rtx_fmt_ee (czero_code, GET_MODE (cond_op0), cond_op0, const0_rtx); + rtx if_then_else + = gen_rtx_IF_THEN_ELSE (mode, czero_cond, const0_rtx, non_zero_op); + rtx set = gen_rtx_SET (target, if_then_else); + + start_sequence (); + rtx_insn *insn = emit_insn (set); + + if (recog_memoized (insn) >= 0) + { + rtx_insn *seq = get_insns (); + end_sequence (); + emit_insn (seq); + + return target; + } + + end_sequence (); + return NULL_RTX; +} + /* Try only simple constants and registers here. More complex cases are handled in noce_try_cmove_arith after noce_try_store_flag_arith has had a go at it. */ @@ -2880,6 +2915,178 @@ noce_try_sign_mask (struct noce_if_info *if_info) return true; } +/* Check if OP is supported by conditional zero based if conversion, + returning TRUE if satisfied otherwise FALSE. + + OP is the operation to check. */ + +static bool +noce_cond_zero_binary_op_supported (rtx op) +{ + enum rtx_code opcode = GET_CODE (op); + + /* Strip SIGN_EXTEND or ZERO_EXTEND if any. */ + if (opcode == SIGN_EXTEND || opcode == ZERO_EXTEND) + opcode = GET_CODE (XEXP (op, 0)); + + if (opcode == PLUS || opcode == MINUS || opcode == IOR || opcode == XOR + || opcode == ASHIFT || opcode == ASHIFTRT || opcode == LSHIFTRT + || opcode == ROTATE || opcode == ROTATERT) + return true; + + return false; +} + +/* Helper function to return REG itself or inner expression of a SUBREG, + otherwise NULL_RTX for other RTX_CODE. */ + +static rtx +get_base_reg (rtx exp) +{ + if (REG_P (exp)) + return exp; + else if (SUBREG_P (exp)) + return SUBREG_REG (exp); + + return NULL_RTX; +} + +/* Check if IF-BB and THEN-BB satisfy the condition for conditional zero + based if conversion, returning TRUE if satisfied otherwise FALSE. + + IF_INFO describes the if-conversion scenario under consideration. + COMMON_PTR points to the common REG of canonicalized IF_INFO->A and + IF_INFO->B. + CZERO_CODE_PTR points to the comparison code to use in czero RTX. + A_PTR points to the A expression of canonicalized IF_INFO->A. + TO_REPLACE points to the RTX to be replaced by czero RTX destnation. */ + +static bool +noce_bbs_ok_for_cond_zero_arith (struct noce_if_info *if_info, rtx *common_ptr, + enum rtx_code *czero_code_ptr, rtx *a_ptr, + rtx **to_replace) +{ + rtx common = NULL_RTX; + rtx cond = if_info->cond; + rtx a = copy_rtx (if_info->a); + rtx b = copy_rtx (if_info->b); + rtx bin_op1 = NULL_RTX; + enum rtx_code czero_code = UNKNOWN; + bool reverse = false; + rtx op0, op1, bin_exp; + + if (!noce_simple_bbs (if_info)) + return false; + + /* COND must be EQ or NE comparision of a reg and 0. */ + if (GET_CODE (cond) != NE && GET_CODE (cond) != EQ) + return false; + if (!REG_P (XEXP (cond, 0)) || !rtx_equal_p (XEXP (cond, 1), const0_rtx)) + return false; + + /* Canonicalize x = y : (y op z) to x = (y op z) : y. */ + if (REG_P (a) && noce_cond_zero_binary_op_supported (b)) + { + std::swap (a, b); + reverse = !reverse; + } + + /* Check if x = (y op z) : y is supported by czero based ifcvt. */ + if (!(noce_cond_zero_binary_op_supported (a) && REG_P (b))) + return false; + + /* Strip sign_extend if any. */ + if (GET_CODE (a) == SIGN_EXTEND || GET_CODE (a) == ZERO_EXTEND) + bin_exp = XEXP (a, 0); + else + bin_exp = a; + + /* Canonicalize x = (z op y) : y to x = (y op z) : y */ + op1 = get_base_reg (XEXP (bin_exp, 1)); + if (op1 && rtx_equal_p (op1, b) && COMMUTATIVE_ARITH_P (bin_exp)) + std::swap (XEXP (bin_exp, 0), XEXP (bin_exp, 1)); + + op0 = get_base_reg (XEXP (bin_exp, 0)); + if (op0 && rtx_equal_p (op0, b)) + { + common = b; + bin_op1 = XEXP (bin_exp, 1); + czero_code = reverse + ? noce_reversed_cond_code (if_info) + : GET_CODE (cond); + } + else + return false; + + if (czero_code == UNKNOWN) + return false; + + if (CONST_INT_P (bin_op1) || REG_P (bin_op1)) + *to_replace = &XEXP (bin_exp, 1); + else if (SUBREG_P (bin_op1)) + *to_replace = &SUBREG_REG (XEXP (bin_exp, 1)); + else + return false; + + *common_ptr = common; + *czero_code_ptr = czero_code; + *a_ptr = a; + + return true; +} + +/* Try to covert if-then-else with conditional zero, + returning TURE on success or FALSE on failure. + IF_INFO describes the if-conversion scenario under consideration. */ + +static int +noce_try_cond_zero_arith (struct noce_if_info *if_info) +{ + rtx target, a; + rtx_insn *seq; + machine_mode mode = GET_MODE (if_info->x); + rtx common = NULL_RTX; + enum rtx_code czero_code = UNKNOWN; + rtx non_zero_op = NULL_RTX; + rtx *to_replace = NULL; + + if (!noce_bbs_ok_for_cond_zero_arith (if_info, &common, &czero_code, &a, + &to_replace)) + return false; + + /* Disallow CONST_INT currently for simplicity*/ + if (to_replace == NULL || !REG_P (*to_replace)) + return false; + + non_zero_op = *to_replace; + + start_sequence (); + + /* If x is used in both input and out like x = c ? x + z : x, + use a new reg to avoid modifying x */ + if (common && rtx_equal_p (common, if_info->x)) + target = gen_reg_rtx (mode); + else + target = if_info->x; + + target = noce_emit_czero (if_info, czero_code, non_zero_op, target); + if (!target || !to_replace) + { + end_sequence (); + return false; + } + + *to_replace = target; + emit_insn (gen_rtx_SET (if_info->x, a)); + + seq = end_ifcvt_sequence (if_info); + if (!seq || !targetm.noce_conversion_profitable_p (seq, if_info)) + return false; + + emit_insn_before_setloc (seq, if_info->jump, INSN_LOCATION (if_info->insn_a)); + if_info->transform_name = "noce_try_cond_zero_arith"; + return true; +} /* Optimize away "if (x & C) x |= C" and similar bit manipulation transformations. */ @@ -3975,6 +4182,9 @@ noce_process_if_block (struct noce_if_info *if_info) goto success; if (noce_try_store_flag_mask (if_info)) goto success; + if (HAVE_conditional_move + && noce_try_cond_zero_arith (if_info)) + goto success; if (HAVE_conditional_move && noce_try_cmove_arith (if_info)) goto success; diff --git a/gcc/testsuite/gcc.target/riscv/zicond_ifcvt_opt.c b/gcc/testsuite/gcc.target/riscv/zicond_ifcvt_opt.c new file mode 100644 index 00000000000..9357f26d978 --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/zicond_ifcvt_opt.c @@ -0,0 +1,682 @@ +/* { dg-do compile } */ +/* { dg-options "-march=rv64gc_zbb_zicond -mabi=lp64d -O2 " } */ +/* { dg-skip-if "" { *-*-* } {"-O0" "-O1" "-Os" "-Og" "-O3" "-Oz" "-flto"} } */ + +long +test_ADD_ceqz (long x, long y, long z, long c) +{ + if (c) + x = y + z; + else + x = y; + return x; +} + +long +test_ADD_ceqz_x (long x, long z, long c) +{ + if (c) + x = x + z; + + return x; +} + +long +test_ADD_nez (long x, long y, long z, long c) +{ + if (c) + x = y; + else + x = y + z; + return x; +} + +long +test_ADD_nez_x (long x, long z, long c) +{ + if (c) + { + } + else + x = x + z; + return x; +} + +long +test_ADD_nez_2 (long x, long y, long z, long c) +{ + if (!c) + x = y + z; + else + x = y; + return x; +} + +long +test_ADD_nez_x_2 (long x, long z, long c) +{ + if (!c) + x = x + z; + + return x; +} + +long +test_ADD_eqz_2 (long x, long y, long z, long c) +{ + if (!c) + x = y; + else + x = y + z; + return x; +} + +long +test_ADD_eqz_x_2 (long x, long z, long c) +{ + if (!c) + { + } + else + x = x + z; + return x; +} + +long +test_SUB_ceqz (long x, long y, long z, long c) +{ + if (c) + x = y - z; + else + x = y; + return x; +} + +long +test_SUB_ceqz_x (long x, long z, long c) +{ + if (c) + x = x - z; + + return x; +} + +long +test_SUB_nez (long x, long y, long z, long c) +{ + if (c) + x = y; + else + x = y - z; + return x; +} + +long +test_SUB_nez_x (long x, long z, long c) +{ + if (c) + { + } + else + x = x - z; + return x; +} + +long +test_SUB_nez_2 (long x, long y, long z, long c) +{ + if (!c) + x = y - z; + else + x = y; + return x; +} + +long +test_SUB_nez_x_2 (long x, long z, long c) +{ + if (!c) + x = x - z; + + return x; +} + +long +test_SUB_eqz_2 (long x, long y, long z, long c) +{ + if (!c) + x = y; + else + x = y - z; + return x; +} + +long +test_SUB_eqz_x_2 (long x, long z, long c) +{ + if (!c) + { + } + else + x = x - z; + return x; +} + +long +test_IOR_ceqz (long x, long y, long z, long c) +{ + if (c) + x = y | z; + else + x = y; + return x; +} + +long +test_IOR_ceqz_x (long x, long z, long c) +{ + if (c) + x = x | z; + + return x; +} + +long +test_IOR_nez (long x, long y, long z, long c) +{ + if (c) + x = y; + else + x = y | z; + return x; +} + +long +test_IOR_nez_x (long x, long z, long c) +{ + if (c) + { + } + else + x = x | z; + return x; +} + +long +test_IOR_nez_2 (long x, long y, long z, long c) +{ + if (!c) + x = y | z; + else + x = y; + return x; +} + +long +test_IOR_nez_x_2 (long x, long z, long c) +{ + if (!c) + x = x | z; + + return x; +} + +long +test_IOR_eqz_2 (long x, long y, long z, long c) +{ + if (!c) + x = y; + else + x = y | z; + return x; +} + +long +test_IOR_eqz_x_2 (long x, long z, long c) +{ + if (!c) + { + } + else + x = x | z; + return x; +} + +long +test_XOR_ceqz (long x, long y, long z, long c) +{ + if (c) + x = y ^ z; + else + x = y; + return x; +} + +long +test_XOR_ceqz_x (long x, long z, long c) +{ + if (c) + x = x ^ z; + + return x; +} + +long +test_XOR_nez (long x, long y, long z, long c) +{ + if (c) + x = y; + else + x = y ^ z; + return x; +} + +long +test_XOR_nez_x (long x, long z, long c) +{ + if (c) + { + } + else + x = x ^ z; + return x; +} + +long +test_XOR_nez_2 (long x, long y, long z, long c) +{ + if (!c) + x = y ^ z; + else + x = y; + return x; +} + +long +test_XOR_nez_x_2 (long x, long z, long c) +{ + if (!c) + x = x ^ z; + + return x; +} + +long +test_XOR_eqz_2 (long x, long y, long z, long c) +{ + if (!c) + x = y; + else + x = y ^ z; + return x; +} + +long +test_XOR_eqz_x_2 (long x, long z, long c) +{ + if (!c) + { + } + else + x = x ^ z; + return x; +} + +long +test_ADD_ceqz_reverse_bin_oprands (long x, long y, long z, long c) +{ + if (c) + x = z + y; + else + x = y; + return x; +} + +long +test_ADD_ceqz_x_reverse_bin_oprands (long x, long z, long c) +{ + if (c) + x = z + x; + + return x; +} + +long +test_ADD_nez_reverse_bin_oprands (long x, long y, long z, long c) +{ + if (c) + x = y; + else + x = z + y; + return x; +} + +long +test_ADD_nez_x_reverse_bin_oprands (long x, long z, long c) +{ + if (c) + { + } + else + x = z + x; + return x; +} + +long +test_ADD_nez_2_reverse_bin_oprands (long x, long y, long z, long c) +{ + if (!c) + x = z + y; + else + x = y; + return x; +} + +long +test_ADD_nez_x_2_reverse_bin_oprands (long x, long z, long c) +{ + if (!c) + x = z + x; + + return x; +} + +long +test_ADD_eqz_2_reverse_bin_oprands (long x, long y, long z, long c) +{ + if (!c) + x = y; + else + x = z + y; + return x; +} + +long +test_ADD_eqz_x_2_reverse_bin_oprands (long x, long z, long c) +{ + if (!c) + { + } + else + x = z + x; + return x; +} + +long +test_IOR_ceqz_reverse_bin_oprands (long x, long y, long z, long c) +{ + if (c) + x = z | y; + else + x = y; + return x; +} + +long +test_IOR_ceqz_x_reverse_bin_oprands (long x, long z, long c) +{ + if (c) + x = z | x; + + return x; +} + +long +test_IOR_nez_reverse_bin_oprands (long x, long y, long z, long c) +{ + if (c) + x = y; + else + x = z | y; + return x; +} + +long +test_IOR_nez_x_reverse_bin_oprands (long x, long z, long c) +{ + if (c) + { + } + else + x = z | x; + return x; +} + +long +test_IOR_nez_2_reverse_bin_oprands (long x, long y, long z, long c) +{ + if (!c) + x = z | y; + else + x = y; + return x; +} + +long +test_IOR_nez_x_2_reverse_bin_oprands (long x, long z, long c) +{ + if (!c) + x = z | x; + + return x; +} + +long +test_IOR_eqz_2_reverse_bin_oprands (long x, long y, long z, long c) +{ + if (!c) + x = y; + else + x = z | y; + return x; +} + +long +test_IOR_eqz_x_2_reverse_bin_oprands (long x, long z, long c) +{ + if (!c) + { + } + else + x = z | x; + return x; +} + +long +test_XOR_ceqz_reverse_bin_oprands (long x, long y, long z, long c) +{ + if (c) + x = z ^ y; + else + x = y; + return x; +} + +long +test_XOR_ceqz_x_reverse_bin_oprands (long x, long z, long c) +{ + if (c) + x = z ^ x; + + return x; +} + +long +test_XOR_nez_reverse_bin_oprands (long x, long y, long z, long c) +{ + if (c) + x = y; + else + x = z ^ y; + return x; +} + +long +test_XOR_nez_x_reverse_bin_oprands (long x, long z, long c) +{ + if (c) + { + } + else + x = z ^ x; + return x; +} + +long +test_XOR_nez_2_reverse_bin_oprands (long x, long y, long z, long c) +{ + if (!c) + x = z ^ y; + else + x = y; + return x; +} + +long +test_XOR_nez_x_2_reverse_bin_oprands (long x, long z, long c) +{ + if (!c) + x = z ^ x; + + return x; +} + +long +test_XOR_eqz_2_reverse_bin_oprands (long x, long y, long z, long c) +{ + if (!c) + x = y; + else + x = z ^ y; + return x; +} + +long +test_XOR_eqz_x_2_reverse_bin_oprands (long x, long z, long c) +{ + if (!c) + { + } + else + x = z ^ x; + return x; +} + +long +test_ShiftLeft_eqz (long x, long y, long z, long c) +{ + if (c) + x = y << z; + else + x = y; + return x; +} + +long +test_ShiftR_eqz (long x, long y, long z, long c) +{ + if (c) + x = y >> z; + else + x = y; + return x; +} + +unsigned long +test_ShiftR_logical_eqz (unsigned long x, unsigned long y, unsigned long z, + unsigned long c) +{ + if (c) + x = y >> z; + else + x = y; + return x; +} + +unsigned long +test_RotateL_eqz (unsigned long x, unsigned long y, unsigned long z, + unsigned long c) +{ + if (c) + x = (y << z) | (y >> (64 - z)); + else + x = y; + return x; +} + +unsigned long +test_RotateR_eqz (unsigned long x, unsigned long y, unsigned long z, + unsigned long c) +{ + if (c) + x = (y >> z) | (y << (64 - z)); + else + x = y; + return x; +} + +int +test_ADD_ceqz_int (int x, int y, int z, int c) +{ + if (c) + x = y + z; + else + x = y; + return x; +} + +int +test_ShiftLeft_eqz_int (int x, int y, int z, int c) +{ + if (c) + x = y << z; + else + x = y; + return x; +} + +int +test_ShiftR_eqz_int (int x, int y, int z, int c) +{ + if (c) + x = y >> z; + else + x = y; + return x; +} + +unsigned int +test_ShiftR_logical_eqz_int (unsigned int x, unsigned int y, unsigned int z, + unsigned int c) +{ + if (c) + x = y >> z; + else + x = y; + return x; +} + +unsigned int +test_RotateL_eqz_int (unsigned int x, unsigned int y, unsigned int z, + unsigned int c) +{ + if (c) + x = (y << z) | (y >> (32 - z)); + else + x = y; + return x; +} + +unsigned int +test_RotateR_eqz_int (unsigned int x, unsigned int y, unsigned int z, + unsigned int c) +{ + if (c) + x = (y >> z) | (y << (32 - z)); + else + x = y; + return x; +} + +/* { dg-final { scan-assembler-times {czero\.eqz} 39 } } */ +/* { dg-final { scan-assembler-times {czero\.nez} 28 } } */
op=[PLUS, MINUS, IOR, XOR, ASHIFT, ASHIFTRT, LSHIFTRT, ROTATE, ROTATERT] SIGN_EXTEND, ZERO_EXTEND and SUBREG has been considered to support SImode in 64-bit machine. Conditional op, if zero rd = (rc == 0) ? (rs1 op rs2) : rs1 --> czero.nez rd, rs2, rc op rd, rs1, rd Conditional op, if non-zero rd = (rc != 0) ? (rs1 op rs2) : rs1 --> czero.eqz rd, rs2, rc op rd, rs1, rd Co-authored-by: Xiao Zeng<zengxiao@eswincomputing.com> gcc/ChangeLog: * ifcvt.cc (noce_try_cond_zero_arith):handler for condtional zero based ifcvt (noce_emit_czero): helper for noce_try_cond_zero_arith (noce_cond_zero_binary_op_supported): check supported OPs for condtional zero based ifcvt (get_base_reg): get base reg of a subreg or the reg itself (noce_bbs_ok_for_cond_zero_arith): check if BBs are OK for condtional zero based ifcvt (noce_process_if_block): add noce_try_cond_zero_arith gcc/testsuite/ChangeLog: * gcc.target/riscv/zicond_ifcvt_opt.c: New test. --- gcc/ifcvt.cc | 210 ++++++ .../gcc.target/riscv/zicond_ifcvt_opt.c | 682 ++++++++++++++++++ 2 files changed, 892 insertions(+) create mode 100644 gcc/testsuite/gcc.target/riscv/zicond_ifcvt_opt.c