"Moore, Catherine" <catherine_mo...@mentor.com> writes: > I'm sorry for wasting your time. I accidentally posted an older version > of the patch earlier this afternoon. > This is the version that I meant to post and is hopefully a lot closer > to what you are looking for. > I named some of the predicates/constraints differently than your current > suggestions, but hopefully they will be OK.
Yeah, that's fine. I'll fold my comments from yesterday into the reply to this patch. > Index: constraints.md > =================================================================== > --- constraints.md (revision 196638) > +++ constraints.md (working copy) > @@ -43,6 +43,9 @@ > (define_register_constraint "b" "ALL_REGS" > "@internal") > > +(define_register_constraint "u" "M16_REGS" > + "@internal") > + > ;; MIPS16 code always calls through a MIPS16 register; see > mips_emit_call_insn > ;; for details. > (define_register_constraint "c" "TARGET_MIPS16 ? M16_REGS > @@ -170,6 +173,41 @@ > (and (match_operand 0 "call_insn_operand") > (match_test "CONSTANT_P (op)"))) > > +(define_constraint "Udb7" > + "@internal > + A decremented unsigned constant of 7 bits." > + (match_operand 0 "db7_operand")) > + > +(define_constraint "Uead" > + "@internal > + A microMIPS encoded ADDIUR2 immediate operand." > + (match_operand 0 "addiur2_operand")) > + > +(define_constraint "Uean" > + "@internal > + A microMIPS encoded andi operand." > + (match_operand 0 "andi16_operand")) s/andi/ANDI/ > +(define_constraint "Uesp" > + "@internal > + A microMIPS encoded ADDIUR1SP operand." > + (match_operand 0 "addiur1sp_operand")) This is "6 bits zero extended and shifted left twice", so the constraint should be "Uuw6" and the predicate should be "uw6_operand". Redundant "". That frees up "Uesp" for something else, so it might be worth using "Uesp" (instead of "Ueim") for ADDIUSP. > +(define_memory_constraint "ZS" > + "@internal > + A microMIPS memory operand for use with the LWSP/SWSP insns." > + (and (match_code "mem") > + (match_operand 0 "lwsp_swsp_operand"))) > + > +(define_memory_constraint "ZT" > + "@internal > + A microMIPS memory operand for use with the LW16 insn." > + (and (match_code "mem") > + (match_operand 0 "lw16_operand"))) As with LWSP/SWSP, we shouldn't need to split the LW16 and SW16 cases. One constraint is enough for both. We only need different constraints if the offsets have different ranges. I think that means one for LW16/SW16, one for LH16/SH16, one for LB16 and one for SB16. > +(define_predicate "sw16_operand" > + (ior (and (match_code "mem") > + (match_test "m16_based_address_p (XEXP (op, 0), mode, > uw4_operand)")) > + (and (match_code "reg") > + (match_test "REGNO (op) == GP_REG_FIRST")))) I don't understand the "reg" case. The operand is used to match the destination of a store, so it really should be a "mem" in all cases. Same for the other stores. > +(define_predicate "db4_operand" > + (and (match_code "const_int") > + (match_test "mips_unsigned_immediate_p (INTVAL (op) - 1, 4, 0)"))) +1 rather than -1. > +(define_predicate "addiur1sp_operand" > + (and (match_code "const_int") > + (match_test "mips_unsigned_immediate_p (INTVAL (op), 8, 2)"))) Watch out when renaming this to uw6_operand. It should be "6, 2" rather than "8, 2". > @@ -1131,14 +1151,19 @@ > "") > > (define_insn "*add<mode>3" > - [(set (match_operand:GPR 0 "register_operand" "=d,d") > - (plus:GPR (match_operand:GPR 1 "register_operand" "d,d") > - (match_operand:GPR 2 "arith_operand" "d,Q")))] > + [(set (match_operand:GPR 0 "register_operand" "=!u,d,!u,!ks,d,d,d") > + (plus:GPR (match_operand:GPR 1 "register_operand" "!u,d,!u,0,!ks,!ks,d") > + (match_operand:GPR 2 "arith_operand" > "!u,d,Uead,Usb3,Ueim,Uesp,Q")))] Hmm, not sure about these alternatives. Two have the form d/ks/U... (the last two) and I'm not sure which instruction Usb3 corresponds to. My guess from the manual would be: [(set (match_operand:GPR 0 "register_operand" "=!u,d,!u,!u,!ks,!d,d") (plus:GPR (match_operand:GPR 1 "register_operand" "!u,d,!u,!ks,!ks,0,d") (match_operand:GPR 2 "arith_operand" "!u,d,Uead,Uuw6,Ueim,Usb4,Q")))] > @@ -1347,12 +1372,13 @@ > (set_attr "mode" "<UNITMODE>")]) > > (define_insn "sub<mode>3" > - [(set (match_operand:GPR 0 "register_operand" "=d") > - (minus:GPR (match_operand:GPR 1 "register_operand" "d") > - (match_operand:GPR 2 "register_operand" "d")))] > + [(set (match_operand:GPR 0 "register_operand" "=!u,d") > + (minus:GPR (match_operand:GPR 1 "register_operand" "!u,d") > + (match_operand:GPR 2 "register_operand" "!u,d")))] > "" > - "<d>subu\t%0,%1,%2" > +{ return "<d>subu\t%0,%1,%2"; } The change to the last line shouldn't be needed. (Plain "..." is better than { return "...."; } where possible, because the generator doesn't need to create a separate output function.) > @@ -4362,13 +4398,14 @@ > ;; in FP registers (off by default, use -mdebugh to enable). > > (define_insn "*mov<mode>_internal" > - [(set (match_operand:IMOVE32 0 "nonimmediate_operand" > "=d,d,e,d,m,*f,*f,*d,*m,*d,*z,*a,*d,*B*C*D,*B*C*D,*d,*m") > - (match_operand:IMOVE32 1 "move_operand" > "d,Yd,Yf,m,dJ,*d*J,*m,*f,*f,*z,*d,*J*d,*a,*d,*m,*B*C*D,*B*C*D"))] > + [(set (match_operand:IMOVE32 0 "nonimmediate_operand" > "=d,!u,d,e,!u,!u,d,d,ZS,ZU,m,*f,*f,*d,*m,*d,*z,*a,*d,*B*C*D,*B*C*D,*d,*m") > + (match_operand:IMOVE32 1 "move_operand" > "d,J,Yd,Yf,Udb7,ZT,ZS,m,ks,!u,dJ,*d*J,*m,*f,*f,*z,*d,*J*d,*a,*d,*m,*B*C*D,*B*C*D"))] The d <- ZS alternative (the 7th) should be !ks <- ZS instead. > "!TARGET_MIPS16 > && (register_operand (operands[0], <MODE>mode) > || reg_or_0_operand (operands[1], <MODE>mode))" > { return mips_output_move (operands[0], operands[1]); } > - [(set_attr "move_type" > "move,const,const,load,store,mtc,fpload,mfc,fpstore,mfc,mtc,mtlo,mflo,mtc,fpload,mfc,fpstore") > + [(set_attr "move_type" > "move,move,const,const,load,load,load,load,store,store,store,mtc,fpload,mfc,fpstore,mfc,mtc,mtlo,mflo,mtc,fpload,mfc,fpstore") I think the 5th alternative (!u <- Udb7) should be "const" rather than "load". > +/* Return true if X fits within a signed field of BITS bits that is > + shifted left SHIFT bits before being used. */ Too many spaces before "BITS". > +bool > +mips_signed_immediate_p (unsigned HOST_WIDE_INT x, int bits, int shift = 0) > +{ > + x += 1 << (bits + shift - 1); > + return mips_unsigned_immediate_p (x, bits, shift); > +} Formatting: there should be one extra space of indentantion. > +} > + > + > +/* Return true if X is a legitimate address that conforms to the requirements > + for a microMIPS LWSP or SWSP insn. */ Only one blank line rather than two. Richard