Hi Richard, I've attached an example patch for the add pattern that tries to identify the short microMIPS variants. In your last review, you mentioned that you would like to see the register requirements modeled in the patterns. Do you have any comments on the approach that I'm taking? Thanks, Catherine
> -----Original Message----- > From: Richard Sandiford [mailto:rdsandif...@googlemail.com] > Sent: Thursday, July 19, 2012 8:46 PM > ------------------------------------------------------------------------- > +(define_attr "umips_arith" "no, yes" > + (if_then_else (and (eq_attr "type" "arith") > + (not (eq_attr "mode" "DI")) > + (match_test "umips_three_reg_match0 (PATTERN > (insn))")) > + (const_string "yes") > + (const_string "no"))) > ------------------------------------------------------------------------- > > Well, I suppose this OK for now, but it seems odd to be hiding the difference > from the main patterns. Don't we want IRA+reload to optimise for this > where possible? Or does that produce bad results? > More below. > > Rather than define lots of attributes, please just use [(cond ...)] to set the > default directly: > > (define_attr "umips_short_insn" "no, yes" > (cond [(and (eq_attr "type" "arith") > (not (eq_attr "mode" "DI")) > (match_test "umips_three_reg_match0 (PATTERN (insn))")) > (const_string "yes") > > ...] > (const_string "no"))) > > ------------------------------------------------------------------------- > +/* Return true if the insn has three micromips register operands. */ > +int > +umips_three_reg (rtx insn) > +{ > + rtx op0 = XEXP (insn, 0); > + rtx op1 = XEXP (insn, 1); > + rtx op2 = XEXP (insn, 2); > + > + return (op0 != NULL_RTX > + && op1 != NULL_RTX > + && op2 != NULL_RTX > + && REG_P (op0) > + && M16_REG_P (REGNO (op0)) > + && REG_P (op1) > + && M16_REG_P (REGNO (op1)) > + && REG_P (op2) > + && M16_REG_P (REGNO (op2))); > +} > ------------------------------------------------------------------------- > > This doesn't look right. You pass in a complete PATTERN, > so XEXP (insn, 1) will typically be the SET_SRC (such as a PLUS) > and XEXP (insn, 2) usually isn't valid. > > I think --enable-checking=yes,rtl would have tripped over the op2 > extraction and flagged it as a problem. Could you use that option > when testing the updated patch? > > Same problem with the other predicates. > > Please make the predicates return bool rather than int. > > Like I said above, I'd really prefer to see the register requirements > modelled in the patterns directly. You could use the "enabled" to > prevent the new alternatives from being used by non-MIPS16 code. > > > Richard
short-delay.cl
Description: short-delay.cl
short-delay.patch
Description: short-delay.patch
