[PATCHv2,rs6000] Enable have_cbranchcc4 on rs6000

  Hi,
  The patch enables have_cbrnachcc4 which is a flag in ifcvt.cc to
indicate if branch by CC bits is invalid or not. The new expand pattern
"cbranchcc4" is created which intend to match the pattern defined in
"*cbranch", "*cbranch_2insn" and "*creturn". The operand sequence in
"cbranchcc4" is inline with the definition in gccint. And the operand
sequence doesn't matter in pattern matching. So I think it should work.

  Compared to last version, one new predicate and one new expander are
created.

  Bootstrapped and tested on powerpc64-linux BE and LE with no regressions.
Is this okay for trunk? Any recommendations? Thanks a lot.

ChangeLog
2022-11-17  Haochen Gui <guihaoc@linux.ibm.com>

gcc/
	* config/rs6000/predicates.md (all_branch_comparison_operator): New,
	and includes operators in branch_comparison_operator and
	extra_insn_branch_comparison_operator.
	* config/rs6000/rs6000.md (cbranchcc4): New expand pattern.

gcc/testsuite/
	* gcc.target/powerpc/cbranchcc4.c: New.


patch.diff
  

On Thu, Nov 17, 2022 at 1:39 AM HAO CHEN GUI <guihaoc@linux.ibm.com> wrote:

> Hi,
>   The patch enables have_cbrnachcc4 which is a flag in ifcvt.cc to
> indicate if branch by CC bits is invalid or not. The new expand pattern
> "cbranchcc4" is created which intend to match the pattern defined in
> "*cbranch", "*cbranch_2insn" and "*creturn". The operand sequence in
> "cbranchcc4" is inline with the definition in gccint. And the operand
> sequence doesn't matter in pattern matching. So I think it should work.
>
>   Compared to last version, one new predicate and one new expander are
> created.
>
>   Bootstrapped and tested on powerpc64-linux BE and LE with no regressions.
> Is this okay for trunk? Any recommendations? Thanks a lot.
>
> ChangeLog
> 2022-11-17  Haochen Gui <guihaoc@linux.ibm.com>
>
> gcc/
>         * config/rs6000/predicates.md (all_branch_comparison_operator):
> New,
>         and includes operators in branch_comparison_operator and
>         extra_insn_branch_comparison_operator.
>         * config/rs6000/rs6000.md (cbranchcc4): New expand pattern.
>
> gcc/testsuite/
>         * gcc.target/powerpc/cbranchcc4.c: New.
>
>
> patch.diff
> diff --git a/gcc/config/rs6000/predicates.md
> b/gcc/config/rs6000/predicates.md
> index b1fcc69bb60..843b6f39b84 100644
> --- a/gcc/config/rs6000/predicates.md
> +++ b/gcc/config/rs6000/predicates.md
> @@ -1308,6 +1308,7 @@ (define_special_predicate "equality_operator"
>
>  ;; Return 1 if OP is a comparison operation that is valid for a branch
>  ;; instruction.  We check the opcode against the mode of the CC value.
> +
>  ;; validate_condition_mode is an assertion.
>  (define_predicate "branch_comparison_operator"
>     (and (match_operand 0 "comparison_operator")
> @@ -1331,6 +1332,11 @@ (define_predicate
> "extra_insn_branch_comparison_operator"
>                                               GET_MODE (XEXP (op, 0))),
>                      1")))
>
> +;; Return 1 if OP is a comparison operation that is valid for a branch.
> +(define_predicate "all_branch_comparison_operator"
> +   (ior (match_operand 0 "branch_comparison_operator")
> +       (match_operand 0 "extra_insn_branch_comparison_operator")))
> +
>  ;; Return 1 if OP is an unsigned comparison operator.
>  (define_predicate "unsigned_comparison_operator"
>    (match_code "ltu,gtu,leu,geu"))
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index e9e5cd1e54d..7b7d747a85d 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -13067,6 +13067,16 @@ (define_insn_and_split "*<code><mode>_cc"
>  ;; Conditional branches.
>  ;; These either are a single bc insn, or a bc around a b.
>
> +(define_expand "cbranchcc4"
> +  [(set (pc)
> +       (if_then_else (match_operator 0 "all_branch_comparison_operator"
> +                       [(match_operand 1 "cc_reg_operand")
> +                        (match_operand 2 "zero_constant")])
> +                     (label_ref (match_operand 3))
> +                     (pc)))]
> +  ""
> +  "")
> +
>

This is better, but the pattern should be near and after the existing
cbranch<mode>4 patterns earlier in the file, not the *cbranch pattern.  It
doesn't match the comment.

Why are you using zero_constant predicate instead of matching (const_int 0)
for operand 2?

Why does this need the new all_branch_comparison_operator?  Can the ifcvt
optimization correctly elide the 2 insn sequence?

Thanks, David


>  (define_insn "*cbranch"
>    [(set (pc)
>         (if_then_else (match_operator 1 "branch_comparison_operator"
> diff --git a/gcc/testsuite/gcc.target/powerpc/cbranchcc4.c
> b/gcc/testsuite/gcc.target/powerpc/cbranchcc4.c
> new file mode 100644
> index 00000000000..528ba1a878d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/cbranchcc4.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-rtl-ce1" } */
> +/* { dg-final { scan-rtl-dump "noce_try_store_flag_constants" "ce1" } } */
> +
> +/* The inner branch should be detected by ifcvt then be converted to a
> setcc
> +   with a plus by noce_try_store_flag_constants.  */
> +
> +int test (unsigned int a, unsigned int b)
> +{
> +    return (a < b ? 0 : (a > b ? 2 : 1));
> +}
>
  

Hi David,

在 2022/11/17 21:24, David Edelsohn 写道:
> This is better, but the pattern should be near and after the existing cbranch<mode>4 patterns earlier in the file, not the *cbranch pattern.  It doesn't match the comment.
Sure, I will put it after existing "cbranch<mode>4" patterns.

> 
> Why are you using zero_constant predicate instead of matching (const_int 0) for operand 2?
The "const_int 0" is an operand other than a predicate. We need a predicate here.

> 
> Why does this need the new all_branch_comparison_operator?  Can the ifcvt optimization correctly elide the 2 insn sequence?
Because rs6000 defines "*cbranch_2insn" insn, such insns are generated after expand.

(jump_insn 50 47 51 11 (set (pc)
        (if_then_else (ge (reg:CCFP 156)
                (const_int 0 [0]))
            (label_ref 53)
            (pc))) "/home/guihaoc/gcc/gcc-mainline-base/gmp/mpz/cmpabs_d.c":80:7 884 {*cbranch_2insn}
     (expr_list:REG_DEAD (reg:CCFP 156)
        (int_list:REG_BR_PROB 633507684 (nil)))
 -> 53)

In prepare_cmp_insn, the comparison is verified by insn_operand_matches. If
extra_insn_branch_comparison_operator is not included in "cbranchcc4" predicate,
it hits ICE here.

  if (GET_MODE_CLASS (mode) == MODE_CC)
    {
      enum insn_code icode = optab_handler (cbranch_optab, CCmode);
      test = gen_rtx_fmt_ee (comparison, VOIDmode, x, y);
      gcc_assert (icode != CODE_FOR_nothing
                  && insn_operand_matches (icode, 0, test));
      *ptest = test;
      return;
    }

The real conditional move is generated by emit_conditional_move_1. Commonly
"*cbranch_2insn" can't be optimized out and it returns NULL_RTX.

      if (COMPARISON_P (comparison))
        {
          saved_pending_stack_adjust save;
          save_pending_stack_adjust (&save);
          last = get_last_insn ();
          do_pending_stack_adjust ();
          machine_mode cmpmode = comp.mode;
          prepare_cmp_insn (XEXP (comparison, 0), XEXP (comparison, 1),
                            GET_CODE (comparison), NULL_RTX, unsignedp,
                            OPTAB_WIDEN, &comparison, &cmpmode);
          if (comparison)
            {
               rtx res = emit_conditional_move_1 (target, comparison,
                                                  op2, op3, mode);
               if (res != NULL_RTX)
                 return res;
            }
          delete_insns_since (last);
          restore_pending_stack_adjust (&save);

I think that extra_insn_branch_comparison_operator should be included in
"cbranchcc4" predicates as such insns exist. And leave it to
emit_conditional_move which decides whether it can be optimized or not.

Thanks for your comments
Gui Haochen
  

On Fri, Nov 18, 2022 at 02:35:30PM +0800, HAO CHEN GUI wrote:
> 在 2022/11/17 21:24, David Edelsohn 写道:
> > Why are you using zero_constant predicate instead of matching (const_int 0) for operand 2?
> The "const_int 0" is an operand other than a predicate. We need a predicate here.

Said differently, it is passed as an operand to this named pattern or
optab, so you need a match_operand here.

> > Why does this need the new all_branch_comparison_operator?  Can the ifcvt optimization correctly elide the 2 insn sequence?
> Because rs6000 defines "*cbranch_2insn" insn, such insns are generated after expand.
> 
> (jump_insn 50 47 51 11 (set (pc)
>         (if_then_else (ge (reg:CCFP 156)
>                 (const_int 0 [0]))
>             (label_ref 53)
>             (pc))) "/home/guihaoc/gcc/gcc-mainline-base/gmp/mpz/cmpabs_d.c":80:7 884 {*cbranch_2insn}
>      (expr_list:REG_DEAD (reg:CCFP 156)
>         (int_list:REG_BR_PROB 633507684 (nil)))
>  -> 53)

But notice the cost of *cbranch_2insn -- ifcvt should never generate
cbranchcc4 with such composite conditions!

> In prepare_cmp_insn, the comparison is verified by insn_operand_matches. If
> extra_insn_branch_comparison_operator is not included in "cbranchcc4" predicate,
> it hits ICE here.
> 
>   if (GET_MODE_CLASS (mode) == MODE_CC)
>     {
>       enum insn_code icode = optab_handler (cbranch_optab, CCmode);
>       test = gen_rtx_fmt_ee (comparison, VOIDmode, x, y);
>       gcc_assert (icode != CODE_FOR_nothing
>                   && insn_operand_matches (icode, 0, test));
>       *ptest = test;
>       return;
>     }
> 
> The real conditional move is generated by emit_conditional_move_1. Commonly
> "*cbranch_2insn" can't be optimized out and it returns NULL_RTX.
> 
>       if (COMPARISON_P (comparison))
>         {
>           saved_pending_stack_adjust save;
>           save_pending_stack_adjust (&save);
>           last = get_last_insn ();
>           do_pending_stack_adjust ();
>           machine_mode cmpmode = comp.mode;
>           prepare_cmp_insn (XEXP (comparison, 0), XEXP (comparison, 1),
>                             GET_CODE (comparison), NULL_RTX, unsignedp,
>                             OPTAB_WIDEN, &comparison, &cmpmode);
>           if (comparison)
>             {
>                rtx res = emit_conditional_move_1 (target, comparison,
>                                                   op2, op3, mode);
>                if (res != NULL_RTX)
>                  return res;
>             }
>           delete_insns_since (last);
>           restore_pending_stack_adjust (&save);
> 
> I think that extra_insn_branch_comparison_operator should be included in
> "cbranchcc4" predicates as such insns exist. And leave it to
> emit_conditional_move which decides whether it can be optimized or not.

I don't think we should pretend we have any conditional jumps the
machine does not actually have, in cbranchcc4.  When would this ever be
useful?  cror;beq can be quite expensive, compared to the code it would
replace anyway.

If something generates those here (which then ICEs later), that is
wrong, fix *that*?  Is it ifcvt doing it?


Segher
  

On Fri, Nov 18, 2022 at 7:20 AM Segher Boessenkool <
segher@kernel.crashing.org> wrote:

> On Fri, Nov 18, 2022 at 02:35:30PM +0800, HAO CHEN GUI wrote:
> > 在 2022/11/17 21:24, David Edelsohn 写道:
> > > Why are you using zero_constant predicate instead of matching
> (const_int 0) for operand 2?
> > The "const_int 0" is an operand other than a predicate. We need a
> predicate here.
>
> Said differently, it is passed as an operand to this named pattern or
> optab, so you need a match_operand here.
>

Earlier versions of patterns for other targets used (const_int 0), but they
seem to have changed that, so match_operand is needed.

Thanks, David


>
> > > Why does this need the new all_branch_comparison_operator?  Can the
> ifcvt optimization correctly elide the 2 insn sequence?
> > Because rs6000 defines "*cbranch_2insn" insn, such insns are generated
> after expand.
> >
> > (jump_insn 50 47 51 11 (set (pc)
> >         (if_then_else (ge (reg:CCFP 156)
> >                 (const_int 0 [0]))
> >             (label_ref 53)
> >             (pc)))
> "/home/guihaoc/gcc/gcc-mainline-base/gmp/mpz/cmpabs_d.c":80:7 884
> {*cbranch_2insn}
> >      (expr_list:REG_DEAD (reg:CCFP 156)
> >         (int_list:REG_BR_PROB 633507684 (nil)))
> >  -> 53)
>
> But notice the cost of *cbranch_2insn -- ifcvt should never generate
> cbranchcc4 with such composite conditions!
>
> > In prepare_cmp_insn, the comparison is verified by insn_operand_matches.
> If
> > extra_insn_branch_comparison_operator is not included in "cbranchcc4"
> predicate,
> > it hits ICE here.
> >
> >   if (GET_MODE_CLASS (mode) == MODE_CC)
> >     {
> >       enum insn_code icode = optab_handler (cbranch_optab, CCmode);
> >       test = gen_rtx_fmt_ee (comparison, VOIDmode, x, y);
> >       gcc_assert (icode != CODE_FOR_nothing
> >                   && insn_operand_matches (icode, 0, test));
> >       *ptest = test;
> >       return;
> >     }
> >
> > The real conditional move is generated by emit_conditional_move_1.
> Commonly
> > "*cbranch_2insn" can't be optimized out and it returns NULL_RTX.
> >
> >       if (COMPARISON_P (comparison))
> >         {
> >           saved_pending_stack_adjust save;
> >           save_pending_stack_adjust (&save);
> >           last = get_last_insn ();
> >           do_pending_stack_adjust ();
> >           machine_mode cmpmode = comp.mode;
> >           prepare_cmp_insn (XEXP (comparison, 0), XEXP (comparison, 1),
> >                             GET_CODE (comparison), NULL_RTX, unsignedp,
> >                             OPTAB_WIDEN, &comparison, &cmpmode);
> >           if (comparison)
> >             {
> >                rtx res = emit_conditional_move_1 (target, comparison,
> >                                                   op2, op3, mode);
> >                if (res != NULL_RTX)
> >                  return res;
> >             }
> >           delete_insns_since (last);
> >           restore_pending_stack_adjust (&save);
> >
> > I think that extra_insn_branch_comparison_operator should be included in
> > "cbranchcc4" predicates as such insns exist. And leave it to
> > emit_conditional_move which decides whether it can be optimized or not.
>
> I don't think we should pretend we have any conditional jumps the
> machine does not actually have, in cbranchcc4.  When would this ever be
> useful?  cror;beq can be quite expensive, compared to the code it would
> replace anyway.
>
> If something generates those here (which then ICEs later), that is
> wrong, fix *that*?  Is it ifcvt doing it?
>
>
> Segher
>
  

Hi Segher,

在 2022/11/18 20:18, Segher Boessenkool 写道:
> I don't think we should pretend we have any conditional jumps the
> machine does not actually have, in cbranchcc4.  When would this ever be
> useful?  cror;beq can be quite expensive, compared to the code it would
> replace anyway.
> 
> If something generates those here (which then ICEs later), that is
> wrong, fix *that*?  Is it ifcvt doing it?

"*cbranch_2insn" is a valid insn for rs6000. So it generates such insn
at expand pass. The "prepare_cmp_insn" called by ifcvt just wants to verify
that the following comparison rtx is valid.

(unlt (reg:CCFP 156)
    (const_int 0 [0]))

It should be valid as it's extracted from an existing insn. It hits ICE only
when the comparison rtx can't pass the predicate check of "cbranchcc4". So
"cbranchcc4" should include "extra_insn_branch_comparison_operator".

Then, ifcvt tries to call emit_conditional_move_1 to generates a condition
move for FP mode. It definitely fails as there is no conditional move insn for
FP mode in rs6000. The behavior of ifcvt is correct. It tries to do conversion
but fails. It won't hit ICEs after cbranchcc4 is correctly defined.

Actually, "*cbranch_2insn" has the same logical as float "*cbranch" in ifcvt.
Both of them get a final false return from "rs6000_emit_int_cmove" as rs6000
doesn't have conditional move for FP mode.

So I think "cbranchcc4" should include "extra_insn_branch_comparison_operator"
as "*cbranch_2insn" is a valid insn. Just let ifcvt decide a conditional
move is valid or not.

Thanks
Gui Haochen
  

Hi!

On Mon, Nov 21, 2022 at 02:18:39PM +0800, HAO CHEN GUI wrote:
> 在 2022/11/18 20:18, Segher Boessenkool 写道:
> > I don't think we should pretend we have any conditional jumps the
> > machine does not actually have, in cbranchcc4.  When would this ever be
> > useful?  cror;beq can be quite expensive, compared to the code it would
> > replace anyway.
> > 
> > If something generates those here (which then ICEs later), that is
> > wrong, fix *that*?  Is it ifcvt doing it?
> 
> "*cbranch_2insn" is a valid insn for rs6000. So it generates such insn
> at expand pass. The "prepare_cmp_insn" called by ifcvt just wants to verify
> that the following comparison rtx is valid.

*cbranch_2insn is not a machine insn.  It generates a cror and a branch
insn.  This makes no sense to have in a cbranchcc: those do a branch
based on an existing cr field, so based on the *output* of that cror.

If ifcvt requires differently, ifcvt needs fixing.

We want to use the output of the cror multiple times, not generate more
cror insns.

> (unlt (reg:CCFP 156)
>     (const_int 0 [0]))
> 
> It should be valid as it's extracted from an existing insn.

Why is that an argument?  The code is valid, sure, but that doesn't
mean we want to generate it all over the place.

> It hits ICE only
> when the comparison rtx can't pass the predicate check of "cbranchcc4". So
> "cbranchcc4" should include "extra_insn_branch_comparison_operator".
> 
> Then, ifcvt tries to call emit_conditional_move_1 to generates a condition
> move for FP mode. It definitely fails as there is no conditional move insn for
> FP mode in rs6000. The behavior of ifcvt is correct. It tries to do conversion
> but fails. It won't hit ICEs after cbranchcc4 is correctly defined.

I don't think the behaviour of ifcvt is correct here at all, no.  It
also does not consider the cost of the code as far as I can see?  That
could reduce the impact of this problem at least.

> Actually, "*cbranch_2insn" has the same logical as float "*cbranch" in ifcvt.
> Both of them get a final false return from "rs6000_emit_int_cmove" as rs6000
> doesn't have conditional move for FP mode.

I am about to commit patches for that.  But only for p10 and later.
It should eventually work for everything with isel (setbc can often be
optimised to isel after all), but the compiler has to work without isel
as well of course.

> So I think "cbranchcc4" should include "extra_insn_branch_comparison_operator"
> as "*cbranch_2insn" is a valid insn. Just let ifcvt decide a conditional
> move is valid or not.

It makes a bad decision though.  This is not okay.


Segher
  

Hi Haochen,

Thanks for the explanation.

on 2022/11/21 14:18, HAO CHEN GUI wrote:
> Hi Segher,
> 
> 在 2022/11/18 20:18, Segher Boessenkool 写道:
>> I don't think we should pretend we have any conditional jumps the
>> machine does not actually have, in cbranchcc4.  When would this ever be
>> useful?  cror;beq can be quite expensive, compared to the code it would
>> replace anyway.
>>
>> If something generates those here (which then ICEs later), that is
>> wrong, fix *that*?  Is it ifcvt doing it?
> 
> "*cbranch_2insn" is a valid insn for rs6000. So it generates such insn
> at expand pass. The "prepare_cmp_insn" called by ifcvt just wants to verify
> that the following comparison rtx is valid.

Maybe we can adjust prepare_cmp_insn to fail if the constructed cbranchcc4
pattern doesn't satisfy the predicate of operand 0 rather than to assert.
It's something like:

if (!insn_operand_matches (icode, 0, test))
  goto fail;

or only assign and return if insn_operand_matches (icode, 0, test).

The code makes the assumption that all this kind of cbranchcc4 patterns
should match what target defines for cbranchcc4 optab, but unfortunately
it's not sure for our port and I don't see how it should be.

BR,
Kewen
  

Hi Kewen,

在 2022/11/22 11:11, Kewen.Lin 写道:
> Maybe we can adjust prepare_cmp_insn to fail if the constructed cbranchcc4
> pattern doesn't satisfy the predicate of operand 0 rather than to assert.
> It's something like:
> 
> if (!insn_operand_matches (icode, 0, test))
>   goto fail;
> 
> or only assign and return if insn_operand_matches (icode, 0, test).
> 
> The code makes the assumption that all this kind of cbranchcc4 patterns
> should match what target defines for cbranchcc4 optab, but unfortunately
> it's not sure for our port and I don't see how it should be.

Thanks for your comments.

I just drafted a patch to let it go to "fail" when predicate of operand 0 is
not satisfied. It works and passed bootstrap on ppc64le. But my concern is
prepare_cmp_insn is a generic function and is used to create a cmp rtx. It
is not only called by emit_conditional* (finally called by ifcvt) but other
functions (even new functions). If we change the logical in prepare_cmp_insn,
we may lost some potential optimization. After all, the branch_2insn is a valid
insn.

I think the essential of the problem is we want to exclude those comparisons
(from cbranchcc4 used in ifcvt) which need two CC bits. So, we can change the
logical of ifcvt - add an additional check with predicate of operand 0 when
checking the have_cbranchcc4 flag in ifcvt.

What's your opinion?

Thanks
Gui Haochen
  

Hi Haochen,

on 2022/11/22 13:12, HAO CHEN GUI wrote:
> Hi Kewen,
> 
> 在 2022/11/22 11:11, Kewen.Lin 写道:
>> Maybe we can adjust prepare_cmp_insn to fail if the constructed cbranchcc4
>> pattern doesn't satisfy the predicate of operand 0 rather than to assert.
>> It's something like:
>>
>> if (!insn_operand_matches (icode, 0, test))
>>   goto fail;
>>
>> or only assign and return if insn_operand_matches (icode, 0, test).
>>
>> The code makes the assumption that all this kind of cbranchcc4 patterns
>> should match what target defines for cbranchcc4 optab, but unfortunately
>> it's not sure for our port and I don't see how it should be.
> 
> Thanks for your comments.
> 
> I just drafted a patch to let it go to "fail" when predicate of operand 0 is
> not satisfied. It works and passed bootstrap on ppc64le. But my concern is
> prepare_cmp_insn is a generic function and is used to create a cmp rtx. It
> is not only called by emit_conditional* (finally called by ifcvt) but other
> functions (even new functions). If we change the logical in prepare_cmp_insn,
> we may lost some potential optimization. After all, the branch_2insn is a valid
> insn.

I have one assumption that without your proposed have_cbranchcc4 change for
rs6000, for this generic prepare_cmp_insn, it would never be called with CCmode
on rs6000, since we would get ICE with icode CODE_FOR_nothing otherwise.
It means we don't lose anything than before.  Besides, excepting for those
conditional call sites, I doubt CCmode would be used for calling it.  Could
you have a check?

> 
> I think the essential of the problem is we want to exclude those comparisons
> (from cbranchcc4 used in ifcvt) which need two CC bits. So, we can change the
> logical of ifcvt - add an additional check with predicate of operand 0 when
> checking the have_cbranchcc4 flag in ifcvt.

I think that would work.  The only concern is that some use (future) of
prepare_cmp_insn like how it's used in ifcvt would need the same pre checking,
otherwise the ICE happens again.

BR,
Kewen
  

Hi Segher,

Thanks for your comments.

在 2022/11/22 7:49, Segher Boessenkool 写道:
> *cbranch_2insn is not a machine insn.  It generates a cror and a branch
> insn.  This makes no sense to have in a cbranchcc: those do a branch
> based on an existing cr field, so based on the *output* of that cror.
> 
> If ifcvt requires differently, ifcvt needs fixing.
> 
I have a question here.
For rs6000, "*cbranch_2insn" should not be generated by cbranch_optab?
I mean it gets icode from cbranch_optab and generates insn from this
icode. If so, the predicate of cbranchcc4 should be checked every time
before insn generation other than just doing an assertion.

> We want to use the output of the cror multiple times, not generate more
> cror insns.
> 
> I don't think the behaviour of ifcvt is correct here at all, no.  It
> also does not consider the cost of the code as far as I can see?  That
> could reduce the impact of this problem at least.
ifcvt tries to generate the converted sequence. Then it compares the cost
of new sequence to the cost of orginial. If it benefits, the conversion
will be done.

Thanks
Gui Haochen