"Moore, Catherine" <catherine_mo...@mentor.com> writes: > I'm testing a slightly different patch from the one that Steve posted: > > Index: mips.md > =================================================================== > --- mips.md (revision 199648) > +++ mips.md (working copy) > @@ -703,8 +703,13 @@ > > ;; Is it a single instruction? > (define_attr "single_insn" "no,yes" > - (symbol_ref "(get_attr_length (insn) == (TARGET_MIPS16 ? 2 : 4) > - ? SINGLE_INSN_YES : SINGLE_INSN_NO)")) > + (if_then_else (ior (and (match_test "TARGET_MIPS16") > + (match_test "get_attr_length (insn) == 2")) > + (and (eq_attr "compression" "micromips,all") > + (match_test "TARGET_MICROMIPS")) > + (match_test "get_attr_length (insn) == 4")) > + (const_string "yes") > + (const_string "no")))
4 isn't OK for MIPS16 though. There's also the problem that Maciej pointed out: a length of 4 doesn't imply a single insn on microMIPS. E.g. an unsplit doubleword move to or from the accumulator registers is a pair of 2-byte microMIPS instructions, so although its overall length is 4, it isn't a single insn. The original code has the same problem. In practice, the split should have happened by dbr_schedule time, but it seems bad practice to rely on that. (FWIW, the MIPS16 definition comes from the historical attitude that extended instructions count as 2 instructions. The *_insns functions also follow this counting.) I'm going to try redefining the length attribute after: ;; "Ghost" instructions occupy no space. (eq_attr "type" "ghost") (const_int 0) in terms of an "insn_count" attribute. This will conervatively count 4 for each microMIPS instruction in an unsplit multi-instruction sequence, just as we do now. Any attempt to change that should be a separate patch anyway. Thanks, Richard