Hi!
Looks pretty good. Some of it shows its age. Indents are weird in many
places (tabs that should be two spaces; tabs after spaces; eight spaces
instead of a tab; continuation lines aligned wrong). A few more specific
comments (many of those happen more than once):
On Sat, Apr 01, 2017 at 05:47:55PM +0100, Andrew Jenner wrote:
+; Complex modes excpet CQI, which needs special treatment.
Typo ("except").
+(define_code_attr cond_shift
+ [(ashift "1") (lshiftrt "1") (rotate "1") (rotatert "1")
+ (ashiftrt "!CONST_INT_P (operands[2]) || INTVAL (operands[2]) <
GET_MODE_BITSIZE (GET_MODE (operands[0])) - 1")])
That line is a bit long.
+ do {
+ i -= 2;
+ tmp = simplify_gen_subreg (HImode, operands[0], <MODE>mode, i);
+ emit_insn (gen_pushhi1 (tmp));
+ } while (i > 0);
The { } should be on separate lines, and with an extra indent.
+ switch (GET_CODE (zext))
+ {
+ case ZERO_EXTEND:
The case should be indented one less (the same as the {).
+; Combine sometimes doesn't try to split off an outer ZERO_EXTEND, perhaps
+; because the combined insn was made up of only two insns. IMHO, this sucks.
Yes, combine doesn't yet do 2->2 combinations. And yes it sucks.
+ /* For the benefit of the lower-subreg pass, create a HImode subreg if the
+ * zero_extract operand is larger than HImode. */
s/\*/ / on that last line.
+ "INTVAL (operands[2]) % BITS_PER_UNIT == 0
+ && (!MEM_P (operands[0]) || !MEM_P (operands[1]))"
+ "#"
+ "reload_completed"
This should probably be "&& reload_completed".
+(define_insn "*xchg<mode>2"
+ [(set (match_operand:LE16 0 "register_operand" "+<r>")
+ (match_operand:LE16 1 "nonimmediate_operand" "+<r>m"))
+ (set (match_dup 1) (match_dup 0))]
+ "!MEM_P (operands[0]) || !MEM_P (operands[1])"
operands[0] cannot be a MEM anyway?
+; Combine creates such insns. Get rid of the clobber and the scratch register.
+(define_split
+ [(set (reg:SET_SCZ CC_REG)
+ (compare:SET_SCZ (plus:MO (match_operand:MO 0 "nonimmediate_operand")
+ (match_operand:MO 1 "const_int_operand"))
+ (const_int 0)))
+ (clobber (scratch:MO))]
+ "INTVAL (operands[1]) != -32768"
+ [(set (reg:CC CC_REG) (compare:CC (match_dup 0) (match_dup 2)))]
+{
+ operands[2] = GEN_INT (-INTVAL (operands[1]));
+})
Hrm, if you have a testcase, could you create a PR please?
Do you need to special case -32768? You could use gen_int_mode instead.
+; Combine prefers ZERO_EXTEND over AND. Undo this transformation.
+(define_insn_and_split "*zero_extendqihi2"
Combine now does tries the AND form automatically if the ZERO_EXTEND form
does *not* match.
Anyway, looks pretty good :-)
Segher
