Hi Richard, 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. Thanks, Catherine
> -----Original Message----- > From: Richard Sandiford [mailto:rdsandif...@googlemail.com] > Sent: Wednesday, March 20, 2013 5:48 PM > To: Moore, Catherine > Cc: gcc-patches@gcc.gnu.org; Rozycki, Maciej > Subject: Re: FW: [PATCH] [MIPS] microMIPS gcc support > > "Moore, Catherine" <catherine_mo...@mentor.com> writes: > > Index: gcc/config/mips/constraints.md > > > ========================================================== > ========= > > --- gcc/config/mips/constraints.md (revision 196638) > > +++ gcc/config/mips/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 "Uea4" > > + "@internal > > + A microMIPS encoded ADDIUR2 immediate operand." > > + (match_operand 0 "addiur2_operand")) > > I'm still wary of having digits for some "Ue"s and not others, especially > because (as I understand it) ADDIUR2 takes a 3-bit rather than 4-bit operand. > How about "Ueau" here (= add of "u" register). > For consistency... > > > +(define_constraint "Ueim" > > + "@internal > > + A microMIPS encoded ADDIUSP operand." > > + (match_operand 0 "addiusp_operand" "")) > > ...this could be "Ueas" (= add of stack register). Redundant "". > > > +(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 "". > > > +(define_memory_constraint "ZS" > > + "@internal > > + A microMIPS memory operand for use with the LWSP/SWSP insns." > > + (and (match_code "mem") > > + (match_operand 0 "umips_lwsp_swsp_operand"))) > > + > > +(define_memory_constraint "ZT" > > + "@internal > > + A microMIPS memory operand for use with the various LOAD insns." > > + (and (match_code "mem") > > + (match_operand 0 "umips_load_operand"))) > > + > > +(define_memory_constraint "ZU" > > + "@internal > > + A microMIPS memory operand for use with the various STORE insns." > > + (and (match_code "mem") > > + (match_operand 0 "umips_store_operand"))) > > As I mentioned before, we need a separate predicate and constraint for each > range. We can't have things like: > > > + /* For the LBU16 insn. */ > > + if (load && db4_operand (addr.offset, mode)) > > + return true; > > + > > + /* For the SB16 insn. */ > > + if (!load && ub4_operand (addr.offset, mode)) > > + return true; > > + > > + /* For SH16, LHU16, SW16 and LW16 insns. */ return uw4_operand > > + (addr.offset, mode) > > + || uh4_operand (addr.offset, mode); > > because this allows SH16 to be ub4_operand, uw4_operand or uh4_operand, > and so on. There needs to be one predicate/constraint pair for > ub4_operands, one for db4_operands, one for uw4_operands and one for > uh4_operands. > > Please use the function I suggested in my previous reply, in which the offset > predicate is passed in as a function pointer. > > > @@ -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,Uea4,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,Ueau,Uuw6,Ueas,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.) > > > @@ -2878,9 +2905,9 @@ > > ;; register =op1 x > > > > (define_insn "*and<mode>3" > > - [(set (match_operand:GPR 0 "register_operand" "=d,d,d,d,d,d,d") > > - (and:GPR (match_operand:GPR 1 "nonimmediate_operand" > "o,o,W,d,d,d,d") > > - (match_operand:GPR 2 "and_operand" > "Yb,Yh,Yw,K,Yx,Yw,d")))] > > + [(set (match_operand:GPR 0 "register_operand" "=d,d,d,!u,d,d,d,!u,d") > > + (and:GPR (match_operand:GPR 1 "nonimmediate_operand" > "o,o,W,!u,d,d,d,0,d") > > + (match_operand:GPR 2 "and_operand" > > +"Yb,Yh,Yw,Uean,K,Yx,Yw,!u,d")))] > > "!TARGET_MIPS16 && and_operands_ok (<MODE>mode, operands[1], > operands[2])" > > { > > int len; > > @@ -2897,20 +2924,23 @@ > > operands[1] = gen_lowpart (SImode, operands[1]); > > return "lwu\t%0,%1"; > > case 3: > > + case 4: > > return "andi\t%0,%1,%x2"; > > - case 4: > > + case 5: > > len = low_bitmask_len (<MODE>mode, INTVAL (operands[2])); > > operands[2] = GEN_INT (len); > > return "<d>ext\t%0,%1,0,%2"; > > - case 5: > > + case 6: > > return "#"; > > - case 6: > > + case 7: > > + case 8: > > return "and\t%0,%1,%2"; > > default: > > gcc_unreachable (); > > } > > } > > - [(set_attr "move_type" > > "load,load,load,andi,ext_ins,shift_shift,logical") > > + [(set_attr "move_type" > "load,load,load,andi,andi,ext_ins,shift_shift,logical,logical") > > + (set_attr "compression" "*,*,*,micromips,*,*,*,micromips,*") > > (set_attr "mode" "<MODE>")]) > > Looks good. > > > @@ -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,fpl > > + oad,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. > > Richard
short-delay-03.20.patch
Description: short-delay-03.20.patch
