Ping? Thanks! -Zhenqiang
> -----Original Message----- > From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > ow...@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: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
