> -----Original Message-----
> From: Richard Henderson [mailto:r...@redhat.com]
> Sent: Thursday, October 24, 2013 12:14 AM
> To: Zhenqiang Chen; Richard Earnshaw; 'Richard Biener'
> Cc: GCC Patches
> Subject: Re: [PATCH 1/n] Add conditional compare support
>
> > +static enum rtx_code
> > +arm_ccmode_to_code (enum machine_mode mode) {
> > + switch (mode)
> > + {
> > + case CC_DNEmode:
> > + return NE;
>
> Why would you need to encode comparisons in CCmodes?
> That looks like a mis-design to me.
The CCmodes are used to check whether the result of a previous conditional
compare can combine with current compare. By changing it to rtx_code, I can
reuse current code (arm_select_dominance_cc_mode_1) to check it.
The CCmodes are also used to emit the "condition code" for a conditional
execution. E.g.
CC1 (CC_DGEmode) = CCMP (a >= 0, b >= 0)
==> cmp a, #0
cmpge b, #0
CC2 = CCMP ( CC1 != 0, c >= 0)
==> cmpge c, #0
The "ge" is from the mode of CC1 in "CC1 != 0". And the mode of CC2 is not
necessary the same as CC1.
> > +Conditional compare instruction. Operand 2 and 5 are RTLs which
> > +perform two comparisons. Operand 1 is AND or IOR, which operates on
> > +the result of Operand 2 and 5. Operand 0 is the result of operand 1.
> > +
> > +A typical @code{ccmp} pattern looks like
> > +
> > +@smallexample
> > +(define_expand "ccmp"
> > + [(set (match_operand 0 "register_operand" "")
> > + (match_operator 1 ""
> > + [(match_operator:SI 2 "comparison_operator"
> > + [(match_operand:SI 3 "register_operand")
> > + (match_operand:SI 4 "register_operand")])
> > + (match_operator:SI 5 "comparison_operator"
> > + [(match_operand:SI 6 "register_operand")
> > + (match_operand:SI 7 "register_operand")])]))]
> > + ""
> > + "@dots{}")
> > +@end smallexample
>
> This documentation is inadequate. What sorts of input combinations are
> valid?
It depends on target. I will update it with your later comments on
* How to use cmp_hook and ccmp_hook to check valid combinations.
* How to write patterns to support more than two compares.
> How is the middle-end expected to compose more than two compares, as
> you describe in the mail? What mode should the middle-end use to allocate
> that output register?
The ccmp_candidate_p and expand_ccmp_expr_* are using recursively algorithm.
One conditional compare can have two and only two compares. But if the operand
is the result of a previous conditional compare, we have more than two
compares. i.e.
CC1 = CCMP (CMP1, CMP2)
CC2 = CCMP (CC1 != 0, CMP3)
...
CCn = CCMP (CCn-1 != 0, CMPn+1)
And these are the representations we want to have on TREE/GIMPLE after
front-end in my original patch.
http://gcc.1065356.n5.nabble.com/PATCH-1-n-Add-conditional-compare-support-td961953.html
> The only thing that makes a sort of sense to me is something akin to how the
> old cmp + bcc patterns worked. Except that's not especially clean, and
> there's a reason we got rid of them.
>
> I think the only clean interface is going to be a new hook or hooks. The
> return value of the hook with be an rtx akin to the PTEST value returned by
> prepare_cmp_insn. Which is a proper input to cbranch/cstore/etc.
>
> I might think that two hooks would be appropriate because it might make the
> ccmp hook easier. I.e.
>
> res = targetm.cmp_hook(...);
> while (more)
> res = targetm.ccmp_hook(res, ...);
>
> For combinations that can't be implemented with ccmp, the hook can return
> null.
Yip. This will be more efficient. I will update the patch.
In current patch, arm_select_dominance_cc_mode is the "cmp_hook" and
arm_select_dominance_ccmp_mode is the "ccmp_hook".
> > +;; The first compare in this pattern is the result of a previous CCMP.
> > +;; We can not swap it. And we only need its flag.
> > +(define_insn "*ccmp_and"
> > + [(set (match_operand 6 "dominant_cc_register" "")
> > + (compare
> > + (and:SI
> > + (match_operator 4 "expandable_comparison_operator"
> > + [(match_operand 0 "dominant_cc_register" "")
> > + (match_operand:SI 1 "arm_add_operand" "")])
> > + (match_operator:SI 5 "arm_comparison_operator"
> > + [(match_operand:SI 2 "s_register_operand"
> > + "l,r,r,r,r")
> > + (match_operand:SI 3 "arm_add_operand"
> > + "lPy,rI,L,rI,L")]))
> > + (const_int 0)))]
> > + "TARGET_32BIT"
> > + "*
> > + {
> > + static const char *const cmp2[2] =
> > + {
> > + \"cmp%d4\\t%2, %3\",
> > + \"cmn%d4\\t%2, #%n3\"
> > + };
> > + static const char *const ite = \"it\\t%d4\";
> > + static const int cmp_idx[9] = {0, 0, 1, 0, 1};
> > +
> > + if (TARGET_THUMB2) {
> > + output_asm_insn (ite, operands);
> > + }
> > + output_asm_insn (cmp2[cmp_idx[which_alternative]], operands);
> > + return \"\";
> > + }"
>
> I would think simply splitting to a normal cond_exec insn post reload would
> be significantly cleaner than this.
The codes are copied from "*cmp_and ". I will try to simplify to normal
cond_exec insn later.
> And for future reference, don't use '"*', just use '{'. That way you don't
> have
> to escape the embedded quotes, etc.
Thanks. I will update it.
-Zhenqiang
