Re: [PATCH] Add a new conversion for conditional ternary set into ifcvt [PR106536]
Hi!
On Thu, Nov 24, 2022 at 11:24:15AM +0100, Richard Biener wrote:
> On Thu, Nov 24, 2022 at 8:25 AM HAO CHEN GUI wrote:
> > 在 2022/11/24 4:06, Richard Biener 写道:
> > > Wouldn't we usually either add an optab or try to recog a canonical
> > > RTL form instead of adding a new target hook for things like this?
> >
> > Thanks so much for your comments. Please let me make it clear.
> >
> > Do you mean we should create an optab for "setb" pattern (the nested
> > if-then-else insn) and detect candidate insns in ifcvt pass? Then
> > generate the insn with the new optab?
>
> Yes, that would be one way to do it. Another way would be to
> generate a (to be defined) canonical form of such instruction and
> see whether it can be recognized (whether there's a define_insn
> for it).
But these insns are most useful when they come up "naturally", so we
really have to recognise many formulations of it separately.
The machine insn setb x,BF returns:
{ -1, if BF.0 is set
{ 1, if BF.1 is set
{ 0, otherwise
This "otherwise" includes when a floating point comparison returned
"unordered" (BF.3, set if an operand was NaN), and BF.2 ("equal"). If
the comparison was only three-way (integer or fast-math) it is natural
to test for equality first (given we always have exactly one of the
first three BF bits set in that case!)
> Note that were just things that came into my mind here, I'm not too
> familiar with how we handle such situations but at least I'm not
> aware of dozens of target hooks to handle instruction availability.
In similar cases I never could up with anything that worked better than
recognising all possible patterns, unfortunately. We can do a predicate
for that though, there is no need to write it out all over the place :-)
Segher
Re: [PATCH] Add a new conversion for conditional ternary set into ifcvt [PR106536]
Richard Biener via Gcc-patches writes: > On Thu, Nov 24, 2022 at 8:25 AM HAO CHEN GUI wrote: >> >> Hi Richard, >> >> >> 在 2022/11/24 4:06, Richard Biener 写道: >> > Wouldn't we usually either add an optab or try to recog a canonical >> > RTL form instead of adding a new target hook for things like this? >> >> Thanks so much for your comments. Please let me make it clear. >> >> Do you mean we should create an optab for "setb" pattern (the nested >> if-then-else insn) and detect candidate insns in ifcvt pass? Then >> generate the insn with the new optab? > > Yes, that would be one way to do it. Another way would be to > generate a (to be defined) canonical form of such instruction and > see whether it can be recognized (whether there's a define_insn > for it). > > Note that were just things that came into my mind here, I'm not too > familiar with how we handle such situations but at least I'm not > aware of dozens of target hooks to handle instruction availability. Yeah, this was my reaction too. The patch does use recog for the conditional set itself, which is good, but I'm not sure from the patch what the target hook is supposed to do in preparation for the recog. I think it'd be better to avoid having too many hooks that take noce_if_info parameters. It's really just a bunch of internal pass state, rather than something that was designed to be a public interface. Currently we have one hook that takes noce_if_info parameters, for costing. That's still one more than I'd like, but at least there are no correctness concerns and, if someone changes noce_if_info in future, just rebuilding cc1 for the affected targets should be good enough as far as testing goes. If we start using hooks for code generation too, we're effectively distributing the ifcvt pass across targets, which makes the pass harder to maintain. Thanks, Richard
Re: [PATCH] Add a new conversion for conditional ternary set into ifcvt [PR106536]
Hi Nilsson,
在 2022/12/2 10:49, Hans-Peter Nilsson 写道:
> On Wed, 23 Nov 2022, HAO CHEN GUI via Gcc-patches wrote:
>
>> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
>> index 92bda1a7e14..9823eccbe68 100644
>> --- a/gcc/doc/tm.texi
>> +++ b/gcc/doc/tm.texi
>> @@ -7094,6 +7094,15 @@ the @code{POLY_VALUE_MIN}, @code{POLY_VALUE_MAX} and
>> implementation returns the lowest possible value of @var{val}.
>> @end deftypefn
>>
>> +@deftypefn {Target Hook} bool TARGET_NOCE_TERNARY_CSET_P (struct
>> noce_if_info *@var{if_info}, rtx *@var{outer_cond}, rtx *@var{inner_cond},
>> int *@var{int1}, int *@var{int2}, int *@var{int3})
>> +This hook returns true if the if-then-else-join blocks describled in
>
> Random typo spotted: "described"
>
> Also, IMHO needs more explanation (in tm.texi preferably) why
> this doesn't happen as part of general "combine" machinery.
Thanks for your comments. Combine can't take it as the insns are not in same
block. Also combine has the limitation on the number of insns. I will add
those comments.
Thanks
Gui Haochen
>
> brgds, H-P
Re: [PATCH] Add a new conversion for conditional ternary set into ifcvt [PR106536]
On Wed, 23 Nov 2022, HAO CHEN GUI via Gcc-patches wrote:
> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> index 92bda1a7e14..9823eccbe68 100644
> --- a/gcc/doc/tm.texi
> +++ b/gcc/doc/tm.texi
> @@ -7094,6 +7094,15 @@ the @code{POLY_VALUE_MIN}, @code{POLY_VALUE_MAX} and
> implementation returns the lowest possible value of @var{val}.
> @end deftypefn
>
> +@deftypefn {Target Hook} bool TARGET_NOCE_TERNARY_CSET_P (struct
> noce_if_info *@var{if_info}, rtx *@var{outer_cond}, rtx *@var{inner_cond},
> int *@var{int1}, int *@var{int2}, int *@var{int3})
> +This hook returns true if the if-then-else-join blocks describled in
Random typo spotted: "described"
Also, IMHO needs more explanation (in tm.texi preferably) why
this doesn't happen as part of general "combine" machinery.
brgds, H-P
Re: [PATCH] Add a new conversion for conditional ternary set into ifcvt [PR106536]
On Thu, Nov 24, 2022 at 8:25 AM HAO CHEN GUI wrote: > > Hi Richard, > > > 在 2022/11/24 4:06, Richard Biener 写道: > > Wouldn't we usually either add an optab or try to recog a canonical > > RTL form instead of adding a new target hook for things like this? > > Thanks so much for your comments. Please let me make it clear. > > Do you mean we should create an optab for "setb" pattern (the nested > if-then-else insn) and detect candidate insns in ifcvt pass? Then > generate the insn with the new optab? Yes, that would be one way to do it. Another way would be to generate a (to be defined) canonical form of such instruction and see whether it can be recognized (whether there's a define_insn for it). Note that were just things that came into my mind here, I'm not too familiar with how we handle such situations but at least I'm not aware of dozens of target hooks to handle instruction availability. Richard. > My concern is that some candidate insns are target specific. For > example, different modes cause additional zero_extend or subreg insns > generated on different targets. So I put the detection process into a > target hook. > > Looking forward to your advice. > > Thanks again > Gui Haochen
Re: [PATCH] Add a new conversion for conditional ternary set into ifcvt [PR106536]
Hi Richard, 在 2022/11/24 4:06, Richard Biener 写道: > Wouldn't we usually either add an optab or try to recog a canonical > RTL form instead of adding a new target hook for things like this? Thanks so much for your comments. Please let me make it clear. Do you mean we should create an optab for "setb" pattern (the nested if-then-else insn) and detect candidate insns in ifcvt pass? Then generate the insn with the new optab? My concern is that some candidate insns are target specific. For example, different modes cause additional zero_extend or subreg insns generated on different targets. So I put the detection process into a target hook. Looking forward to your advice. Thanks again Gui Haochen
Re: [PATCH] Add a new conversion for conditional ternary set into ifcvt [PR106536]
On Wed, Nov 23, 2022 at 8:09 AM HAO CHEN GUI via Gcc-patches
wrote:
>
> Hi,
> There is a new insn on my target, which has a nested if_then_else and
> set -1, 0 and 1 according to a comparison.
>
>[(set (match_operand:SI 0 "gpc_reg_operand" "=r")
> (if_then_else:SI (lt (match_operand:CC 1 "cc_reg_operand" "y")
> (const_int 0))
> (const_int -1)
> (if_then_else (gt (match_dup 1)
> (const_int 0))
> (const_int 1)
> (const_int 0]
>
> In ifcvt pass, it probably contains a comparison, a branch, a setcc
> and a constant set.
>
> 8: r122:CC=cmp(r120:DI#0,r121:DI#0)
> 9: pc={(r122:CC<0)?L29:pc}
>
>14: r118:SI=r122:CC>0
>
>29: L29:
> 5: r118:SI=0x
>
> This patch adds the new conversion into ifcvt and convert this kind of
> branch into a nested if-then-else insn if the target supports such
> pattern.
>
> HAVE_ternary_conditional_set indicates if the target has such nested
> if-then-else insn. It's set in genconfig. noce_try_ternary_cset will be
> executed to detect suitable pattern and convert it to the nested
> if-then-else insn if HAVE_ternary_conditional_set is set. The hook
> TARGET_NOCE_TERNARY_CSET_P detects target specific pattern and output
> conditions and setting integers for the nested if-then-else.
>
> Bootstrapped and tested on powerpc64-linux BE/LE and x86 with no
> regressions. Is this okay for trunk? Any recommendations? Thanks a lot.
Wouldn't we usually either add an optab or try to recog a canonical
RTL form instead of adding a new target hook for things like this?
> ChangeLog
> 2022-11-23 Haochen Gui
>
> gcc/
> * doc/tm.texi: Regenerate.
> * doc/tm.texi.in (TARGET_NOCE_TERNARY_CSET_P): Document new hook.
> * genconfig.cc (have_ternary_cset_flag): New.
> (walk_insn_part): Detect nested if-then-else with const_int setting
> and set have_ternary_cset_flag.
> (HAVE_ternary_conditional_set): Define.
> * ifcvt.cc (noce_emit_ternary_cset): New function to emit nested
> if-then-else insns.
> (noce_try_ternary_cset): Detect ternary conditional set and emit the
> insn.
> (noce_process_if_block): Try to do ternary condition set convertion
> when a target supports ternary conditional set insn.
> * target.def (noce_ternary_cset_p): New hook.
> * targhooks.cc (default_noce_ternary_cset_p): New function.
> * targhooks.h (default_noce_ternary_cset_p): New declare.
>
>
> patch.diff
> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> index 92bda1a7e14..9823eccbe68 100644
> --- a/gcc/doc/tm.texi
> +++ b/gcc/doc/tm.texi
> @@ -7094,6 +7094,15 @@ the @code{POLY_VALUE_MIN}, @code{POLY_VALUE_MAX} and
> implementation returns the lowest possible value of @var{val}.
> @end deftypefn
>
> +@deftypefn {Target Hook} bool TARGET_NOCE_TERNARY_CSET_P (struct
> noce_if_info *@var{if_info}, rtx *@var{outer_cond}, rtx *@var{inner_cond},
> int *@var{int1}, int *@var{int2}, int *@var{int3})
> +This hook returns true if the if-then-else-join blocks describled in
> +@code{if_info} can be converted to a ternary conditional set implemented by
> +a nested if-then-else insn. The @code{int1}, @code{int2} and @code{int3}
> +are three possible results of the nested if-then-else insn.
> +@code{outer_cond} and @code{inner_cond} are the conditions for outer and
> +if-then-else.
> +@end deftypefn
> +
> @node Scheduling
> @section Adjusting the Instruction Scheduler
>
> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> index 112462310b1..1d6f28cc50a 100644
> --- a/gcc/doc/tm.texi.in
> +++ b/gcc/doc/tm.texi.in
> @@ -4631,6 +4631,8 @@ Define this macro if a non-short-circuit operation
> produced by
>
> @hook TARGET_ESTIMATED_POLY_VALUE
>
> +@hook TARGET_NOCE_TERNARY_CSET_P
> +
> @node Scheduling
> @section Adjusting the Instruction Scheduler
>
> diff --git a/gcc/genconfig.cc b/gcc/genconfig.cc
> index b7c6b48eec6..902c832cf5a 100644
> --- a/gcc/genconfig.cc
> +++ b/gcc/genconfig.cc
> @@ -33,6 +33,7 @@ static int max_recog_operands; /* Largest operand number
> seen. */
> static int max_dup_operands;/* Largest number of match_dup in any insn.
> */
> static int max_clobbers_per_insn;
> static int have_cmove_flag;
> +static int have_ternary_cset_flag;
> static int have_cond_exec_flag;
> static int have_lo_sum_flag;
> static int have_rotate_flag;
> @@ -136,6 +137,12 @@ walk_insn_part (rtx part, int recog_p, int
> non_pc_set_src)
> && GET_CODE (XEXP (part, 1)) == MATCH_OPERAND
> && GET_CODE (XEXP (part, 2)) == MATCH_OPERAND)
> have_cmove_flag = 1;
> + else if (recog_p && non_pc_set_src
> + && GET_CODE (XEXP (part, 1)) == CONST_INT
> + && GET_CODE (XEXP (part, 2)) == IF_THEN_ELSE
> +
[PATCH] Add a new conversion for conditional ternary set into ifcvt [PR106536]
Hi,
There is a new insn on my target, which has a nested if_then_else and
set -1, 0 and 1 according to a comparison.
[(set (match_operand:SI 0 "gpc_reg_operand" "=r")
(if_then_else:SI (lt (match_operand:CC 1 "cc_reg_operand" "y")
(const_int 0))
(const_int -1)
(if_then_else (gt (match_dup 1)
(const_int 0))
(const_int 1)
(const_int 0]
In ifcvt pass, it probably contains a comparison, a branch, a setcc
and a constant set.
8: r122:CC=cmp(r120:DI#0,r121:DI#0)
9: pc={(r122:CC<0)?L29:pc}
14: r118:SI=r122:CC>0
29: L29:
5: r118:SI=0x
This patch adds the new conversion into ifcvt and convert this kind of
branch into a nested if-then-else insn if the target supports such
pattern.
HAVE_ternary_conditional_set indicates if the target has such nested
if-then-else insn. It's set in genconfig. noce_try_ternary_cset will be
executed to detect suitable pattern and convert it to the nested
if-then-else insn if HAVE_ternary_conditional_set is set. The hook
TARGET_NOCE_TERNARY_CSET_P detects target specific pattern and output
conditions and setting integers for the nested if-then-else.
Bootstrapped and tested on powerpc64-linux BE/LE and x86 with no
regressions. Is this okay for trunk? Any recommendations? Thanks a lot.
ChangeLog
2022-11-23 Haochen Gui
gcc/
* doc/tm.texi: Regenerate.
* doc/tm.texi.in (TARGET_NOCE_TERNARY_CSET_P): Document new hook.
* genconfig.cc (have_ternary_cset_flag): New.
(walk_insn_part): Detect nested if-then-else with const_int setting
and set have_ternary_cset_flag.
(HAVE_ternary_conditional_set): Define.
* ifcvt.cc (noce_emit_ternary_cset): New function to emit nested
if-then-else insns.
(noce_try_ternary_cset): Detect ternary conditional set and emit the
insn.
(noce_process_if_block): Try to do ternary condition set convertion
when a target supports ternary conditional set insn.
* target.def (noce_ternary_cset_p): New hook.
* targhooks.cc (default_noce_ternary_cset_p): New function.
* targhooks.h (default_noce_ternary_cset_p): New declare.
patch.diff
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 92bda1a7e14..9823eccbe68 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -7094,6 +7094,15 @@ the @code{POLY_VALUE_MIN}, @code{POLY_VALUE_MAX} and
implementation returns the lowest possible value of @var{val}.
@end deftypefn
+@deftypefn {Target Hook} bool TARGET_NOCE_TERNARY_CSET_P (struct noce_if_info
*@var{if_info}, rtx *@var{outer_cond}, rtx *@var{inner_cond}, int *@var{int1},
int *@var{int2}, int *@var{int3})
+This hook returns true if the if-then-else-join blocks describled in
+@code{if_info} can be converted to a ternary conditional set implemented by
+a nested if-then-else insn. The @code{int1}, @code{int2} and @code{int3}
+are three possible results of the nested if-then-else insn.
+@code{outer_cond} and @code{inner_cond} are the conditions for outer and
+if-then-else.
+@end deftypefn
+
@node Scheduling
@section Adjusting the Instruction Scheduler
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 112462310b1..1d6f28cc50a 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -4631,6 +4631,8 @@ Define this macro if a non-short-circuit operation
produced by
@hook TARGET_ESTIMATED_POLY_VALUE
+@hook TARGET_NOCE_TERNARY_CSET_P
+
@node Scheduling
@section Adjusting the Instruction Scheduler
diff --git a/gcc/genconfig.cc b/gcc/genconfig.cc
index b7c6b48eec6..902c832cf5a 100644
--- a/gcc/genconfig.cc
+++ b/gcc/genconfig.cc
@@ -33,6 +33,7 @@ static int max_recog_operands; /* Largest operand number
seen. */
static int max_dup_operands;/* Largest number of match_dup in any insn. */
static int max_clobbers_per_insn;
static int have_cmove_flag;
+static int have_ternary_cset_flag;
static int have_cond_exec_flag;
static int have_lo_sum_flag;
static int have_rotate_flag;
@@ -136,6 +137,12 @@ walk_insn_part (rtx part, int recog_p, int non_pc_set_src)
&& GET_CODE (XEXP (part, 1)) == MATCH_OPERAND
&& GET_CODE (XEXP (part, 2)) == MATCH_OPERAND)
have_cmove_flag = 1;
+ else if (recog_p && non_pc_set_src
+ && GET_CODE (XEXP (part, 1)) == CONST_INT
+ && GET_CODE (XEXP (part, 2)) == IF_THEN_ELSE
+ && GET_CODE (XEXP (XEXP (part, 2), 1)) == CONST_INT
+ && GET_CODE (XEXP (XEXP (part, 2), 2)) == CONST_INT)
+ have_ternary_cset_flag = 1;
break;
case COND_EXEC:
@@ -328,6 +335,11 @@ main (int argc, const char **argv)
else
printf ("#define HAVE_conditional_move 0\n");
+ if (have_ternary_cset_flag)
+printf ("#define HAVE_ternary_conditional_set 1\n");
+ else
+printf
