RE: [Ping] [PATCH, 10/10] aarch64: Handle ccmp in ifcvt to make it work with cmov

[Zhenqiang Chen]

> -----Original Message-----
> From: Richard Henderson [mailto:r...@redhat.com]
> Sent: Sunday, October 12, 2014 5:40 AM
> To: Zhenqiang Chen; gcc-patches@gcc.gnu.org
> Subject: Re: [Ping] [PATCH, 10/10] aarch64: Handle ccmp in ifcvt to make
it
> work with cmov
> 
> On 09/22/2014 11:46 PM, Zhenqiang Chen wrote:
> > @@ -2375,10 +2387,21 @@ noce_get_condition (rtx_insn *jump, rtx_insn
> **earliest, bool then_else_reversed
> >        return cond;
> >      }
> >
> > +  /* For conditional compare, set ALLOW_CC_MODE to TRUE.  */
> > +  if (targetm.gen_ccmp_first)
> > +    {
> > +      rtx prev = prev_nonnote_nondebug_insn (jump);
> > +      if (prev
> > +     && NONJUMP_INSN_P (prev)
> > +     && BLOCK_FOR_INSN (prev) == BLOCK_FOR_INSN (jump)
> > +     && ccmp_insn_p (prev))
> > +   allow_cc_mode = true;
> > +    }
> > +
> >    /* Otherwise, fall back on canonicalize_condition to do the dirty
> >       work of manipulating MODE_CC values and COMPARE rtx codes.  */
> >    tmp = canonicalize_condition (jump, cond, reverse, earliest,
> > -                           NULL_RTX, false, true);
> > +                           NULL_RTX, allow_cc_mode, true);
> 
> This needs a lot more explanation.  Why it it ok to allow a cc_mode when
the
> source is a ccmp, and not for any other comparison?
> 
> The issue is going to be how we use the comparison once we've finished
with
> the transformation.  Is it going to be able to be properly handled by
> emit_conditional_move?
> 
> If the target doesn't have cbranchcc4, I think that prep_cmp_insn will
fail.
> But as you show from

Good point. It is not ccmp special. It is cbranchcc4 related. If I
understand correct, without cbranchcc4, we need put the result to  a tmp
register and generate additional compares, which is not good for
performance.

I update the patch to check

+#if HAVE_cbranchcc4
+  allow_cc_mode = true;
+#endif

Thanks!
-Zhenqiang

> > +++ b/gcc/config/aarch64/aarch64.md
> > @@ -2589,15 +2589,19 @@
> >                        (match_operand:ALLI 3 "register_operand" "")))]
> >    ""
> >    {
> > -    rtx ccreg;
> >      enum rtx_code code = GET_CODE (operands[1]);
> >
> >      if (code == UNEQ || code == LTGT)
> >        FAIL;
> >
> > -    ccreg = aarch64_gen_compare_reg (code, XEXP (operands[1], 0),
> > -                             XEXP (operands[1], 1));
> > -    operands[1] = gen_rtx_fmt_ee (code, VOIDmode, ccreg, const0_rtx);
> > +    if (!ccmp_cc_register (XEXP (operands[1], 0),
> > +                      GET_MODE (XEXP (operands[1], 0))))
> > +      {
> > +   rtx ccreg;
> > +   ccreg = aarch64_gen_compare_reg (code, XEXP (operands[1], 0),
> > +                                    XEXP (operands[1], 1));
> > +   operands[1] = gen_rtx_fmt_ee (code, VOIDmode, ccreg, const0_rtx);
> > +      }
> 
> this change, even more than that may be required.
> 
> 
> r~

10-ifcvt.patch
Description: Binary data