[PATCH 1/n] Add conditional compare support

[Zhenqiang Chen]

> -----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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ccmp-hook4.patch
Type: application/octet-stream
Size: 29482 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20131106/f083b654/attachment.obj>