This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PING] [PATCH 1/n] Add conditional compare support


Ping?

Thanks!
-Zhenqiang

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Zhenqiang Chen
> Sent: Wednesday, November 06, 2013 3:39 PM
> To: 'Richard Henderson'
> Cc: Richard Earnshaw; 'Richard Biener'; GCC Patches
> Subject: RE: [PATCH 1/n] Add conditional compare support
> 
> 
> > -----Original Message-----
> > From: Richard Henderson [mailto:rth@redhat.com]
> > Sent: Tuesday, November 05, 2013 4:39 AM
> > To: Zhenqiang Chen
> > Cc: Richard Earnshaw; 'Richard Biener'; GCC Patches
> > Subject: Re: [PATCH 1/n] Add conditional compare support
> >
> > On 11/04/2013 08:00 PM, Zhenqiang Chen wrote:
> > > Thanks. I add a new hook. The default function will return -1 if the
> > > target does not care about the order.
> > >
> > > +DEFHOOK
> > > +(select_ccmp_cmp_order,
> > > + "For some target (like ARM), the order of two compares is
> > > +sensitive for\n\ conditional compare.  cmp0-cmp1 might be an
> > > +invalid combination.  But when\n\ swapping the order, cmp1-cmp0 is
> valid.
> > > +The function will return\n\
> > > +  -1: if @code{code1} and @code{code2} are valid combination.\n\
> > > +   1: if @code{code2} and @code{code1} are valid combination.\n\
> > > +   0: both are invalid.",
> > > + int, (int code1, int code2),
> > > + default_select_ccmp_cmp_order)
> >
> > Fair enough.  I'd originally been thinking that returning a tri-state
> > value akin to the comparison callback to qsort would allow easy
> > sorting of a whole list of comparisons.  But probably just as easy to
> > open-code while checking for invalid combinations.
> >
> > Checking for invalid while sorting means that we can then disallow
> > returning NULL from the other two hooks.  Because the backend has
> > already had a chance to indicate failure.
> 
> The check is only for the first two compares. And the following compares are
> not checked. In addition, backend might check more staffs (e.g.
> arm_select_dominance_cc_mode) to generate a valid compare instruction.
> 
> > > For gen_ccmp_next, I add another parameter CC_P to indicate the
> > > result is used as CC or not. If CC_P is false, the gen_ccmp_next
> > > will return a general register. This is for code like
> > >
> > > int test (int a, int b)
> > > {
> > >   return a > 0 && b > 0;
> > > }
> > > During expand, there might have no branch at all. So gen_ccmp_next
> > > can not return CC for "a > 0 && b > 0".
> >
> > Uh, no, this is a terrible idea.  There's no need for gen_ccmp_next to
> > re-do the work of cstore_optab.
> >
> > I believe you can use emit_store_flag as a high-level interface here,
> > since there are technically vagaries due to STORE_FLAG_VALUE.  If that
> > turns out to crash or fail in some way, we can talk about using
> > cstore_optab directly given some restrictions.
> 
> emit_store_flag does too much checks. I use cstore_optab to emit the insn.
> 
> +      icode = optab_handler (cstore_optab, CCmode);
> +      if (icode != CODE_FOR_nothing)
> +	{
> +	  rtx target = gen_reg_rtx (word_mode);
> +	  tmp = emit_cstore (target, icode, NE, CCmode, CCmode,
> +			     0, tmp, const0_rtx, 1, word_mode);
> +	  if (tmp)
> +	    return tmp;
> +	}
> 
> > It also means that you shouldn't need all of and_scc_scc, ior_scc_scc,
> > ccmp_and_scc_scc, ccmp_ior_scc_scc.
> 
> Yes. We only need ccmp_and and ccmp_ior now.
> 
> I will verify to remove the existing and_scc_scc, ior_scc_scc,
> and_scc_scc_cmp, ior_scc_scc_cmp once conditional compare is enabled.
> 
> > Although I don't see cstorecc4 defined for ARM, so there is something
> > missing.
> 
> cstorecc4 is added.
> 
> > > +static int
> > > +arm_select_ccmp_cmp_order (int cond1, int cond2) {
> > > +  if (cond1 == cond2)
> > > +    return -1;
> > > +  if (comparison_dominates_p ((enum rtx_code) cond1, (enum
> > > +rtx_code)
> > cond2))
> > > +    return 1;
> > > +  if (comparison_dominates_p ((enum rtx_code) cond2, (enum
> > > + rtx_code)
> > cond1))
> > > +    return -1;
> > > +  return 0;
> > > +
> > > +}
> >
> > This sort does not look stable.  In particular,
> >
> >   if (cond1 == cond2)
> >     return 1;
> >
> > would seem to better preserve the original order of the comparisons.
> 
> -1 is to keep the original order. Anyway I change the function as:
> 
> +/* COND1 and COND2 should be enum rtx_code, which represent two
> compares.
> +   There are order sensitive for conditional compare.  It returns
> +      1: Keep current order.
> +     -1: Swap the two compares.
> +      0: Invalid combination.  */
> +
> +static int
> +arm_select_ccmp_cmp_order (int cond1, int cond2) {
> +  /* THUMB1 does not support conditional compare.  */
> +  if (TARGET_THUMB1)
> +    return 0;
> +
> +  if (cond1 == cond2)
> +    return 1;
> +  if (comparison_dominates_p ((enum rtx_code) cond1, (enum rtx_code)
> cond2))
> +    return -1;
> +  if (comparison_dominates_p ((enum rtx_code) cond2, (enum rtx_code)
> cond1))
> +    return 1;
> +
> +  return 0;
> +}
> 
> Thanks!
> -Zhenqiang




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]