> -----Original Message----- > From: Richard Henderson [mailto:r...@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
ccmp-hook4.patch
Description: Binary data
