[PATCH 1/n] Add conditional compare support
Zhenqiang Chen
zhenqiang.chen@arm.com
Thu Oct 31 09:40:00 GMT 2013
> -----Original Message-----
> From: Richard Henderson [mailto:rth@redhat.com]
> Sent: Thursday, October 31, 2013 4:14 AM
> To: Zhenqiang Chen
> Cc: Richard Earnshaw; 'Richard Biener'; GCC Patches
> Subject: Re: [PATCH 1/n] Add conditional compare support
>
> > +/* RCODE0, RCODE1 and a valid return value should be enum rtx_code.
> > + TCODE should be enum tree_code.
> > + Check whether two compares are a valid combination in the target to
> generate
> > + a conditional compare. If valid, return the new compare after
> combination.
> > + */
> > +DEFHOOK
> > +(legitimize_cmp_combination,
> > + "This target hook returns the dominance compare if the two compares
> > +are\n\ a valid combination. This target hook is required only when
> > +the target\n\ supports conditional compare, such as ARM.", int, (int
> > +rcode0, int rcode1, int tcode),
> > + default_legitimize_cmp_combination)
> > +
> > +/* RCODE0, RCODE1 and a valid return value should be enum rtx_code.
> > + TCODE should be enum tree_code.
> > + Check whether two compares are a valid combination in the target to
> generate
> > + a conditional compare. If valid, return the new compare after
> combination.
> > + The difference from legitimize_cmp_combination is that its first
> compare is
> > + the result of a previous conditional compare, which leads to more
> constrain
> > + on it, since no way to swap the two compares. */ DEFHOOK
> > +(legitimize_ccmp_combination, "This target hook returns the
> > +dominance compare if the two compares are\n\ a valid combination.
> > +This target hook is required only when the target\n\ supports
> > +conditional compare, such as ARM.", int, (int rcode0, int rcode1,
> > +int tcode),
> > + default_legitimize_ccmp_combination)
> > +
>
> Why do these hooks still exist? They should be redundant with ...
They are not necessary. Will remove them.
> > +/* CMP0 and CMP1 are two compares. USED_AS_CC_P indicates whether
> the target
> > + is used as CC or not. TCODE should be enum tree_code.
> > + The hook will return a condtional compare RTX if all checkes are
> > +OK. */ DEFHOOK (gen_ccmp_with_cmp_cmp, "This target hook returns a
> > +condtional compare RTX if the two compares are\n\ a valid
> > +combination. This target hook is required only when the target\n\
> > +supports conditional compare, such as ARM.", rtx, (gimple cmp0,
> > +gimple cmp1, int tcode, bool used_as_cc_p),
> > + default_gen_ccmp_with_cmp_cmp)
> > +
> > +/* CC is the result of a previous conditional compare. CMP1 is a compare.
> > + USED_AS_CC_P indicates whether the target is used as CC or not.
> > + TCODE should be enum tree_code.
> > + The hook will return a condtional compare rtx if all checkes are
> > +OK. */ DEFHOOK (gen_ccmp_with_ccmp_cmp, "This target hook returns
> a
> > +condtional compare RTX if the CC and CMP1 are\n\ a valid combination.
> > +This target hook is required only when the target\n\ supports
> > +conditional compare, such as ARM.", rtx, (rtx cc, gimple cmp1, int
> > +tcode, bool used_as_cc_p),
> > + default_gen_ccmp_with_ccmp_cmp)
> > +
>
> ... these.
>
> Why in the world are you passing down gimple to the backends? The
> expand_operands done therein should have been done in expr.c.
> The hooks are still not what I suggested, particularly
> gen_ccmp_with_cmp_cmp is what I was trying to avoid, passing down two
> initial compares like that.
>
> To be 100% explicit this time, I think the hooks should be
Thank you very much!
> DEFHOOK
> (gen_ccmp_first,
> "This function emits a comparison insn for the first of a sequence of\n\
> conditional comparisions. It returns a comparison expression appropriate\n\
> for passing to @code{gen_ccmp_next} or to @code{cbranch_optab}.", rtx,
> (int code, rtx op0, rtx op1),
> NULL)
> DEFHOOK
> (gen_ccmp_next,
> "This function emits a conditional comparison within a sequence of\n\
> conditional comparisons. The @code{prev} expression is the result of a\n\
> prior call to @code{gen_ccmp_first} or @code{gen_ccmp_next}. It may
> return\n\ @code{NULL} if the combination of @code{prev} and this
> comparison is\n\ not supported, otherwise the result must be appropriate
> for passing to @code{gen_ccmp_next} or @code{cbranch_optab}.", rtx, (rtx
> prev, int code, rtx op0, rtx op1),
> NULL)
gen_ccmp_first and gen_ccmp_next are better. But need some improvement.
With two compares, we might swap the order of the two compares to get a valid combination. This is what current ARM does (Please refer arm_select_dominace_cc_mode, cmp_and, cmp_ior, and_scc_scc and ior_scc_scc). To improve gen_ccmp_first, we need another function/hook to determine which compare is done first. I will double check ARM backend to avoid hook.
Hook gen_ccmp_next needs one more parameter to indicate AND/IOR since they will generate different instructions.
I will update the patch.
> All of your existing tests for HAVE_ccmp should be replaced with
>
> if (targetm.gen_ccmp_first == NULL)
> return; /* No ccmp supported. */
> gcc_checking_assert(targetm.gen_ccmp_next != NULL);
Thanks. I will update it.
-Zhenqiang
More information about the Gcc-patches
mailing list