The patch implements microMIPS32r6 and microMIPS64r6 RECIP.fmt, RINT.fmt, ROUND.L.fmt, ROUND.W.fmt, SEL.fmt, SELEQZ.fmt, SELNEQZ.fmt and CLASS.fmt FPU instructions.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
If you're using the web interface to create the review, please remember to include the full context as described at http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface
My main concern is the removal of several FPU instructions from 32R6 and 64R6.
lib/Target/Mips/MicroMips32r6InstrInfo.td | ||
---|---|---|
115 ↗ | (On Diff #37847) | RECIP -> RECIP_D |
117 ↗ | (On Diff #37847) | RINT -> RINT_D |
120 ↗ | (On Diff #37847) | ROUND_L -> ROUND_L_D |
124 ↗ | (On Diff #37847) | ROUND_W -> ROUND_W_D |
127 ↗ | (On Diff #37847) | SEL -> SEL_D |
714–725 ↗ | (On Diff #37847) | I should have mentioned this sooner but ABSS_FT_MMR6_DESC_BASE seems to have become a generic 2-operand description. This being the case, could you rename it (in a follow-up patch)? It seems strange to see RECIP.S (etc.) described as being an absolute operation. |
716 ↗ | (On Diff #37847) | RECIP -> RECIP_D |
720 ↗ | (On Diff #37847) | ROUND_L -> ROUND_L_D |
724 ↗ | (On Diff #37847) | ROUND_W -> ROUND_W_D |
728 ↗ | (On Diff #37847) | SEL -> SEL_D |
738 ↗ | (On Diff #37847) | RINT -> RINT_D |
1053 ↗ | (On Diff #37847) | RECIP -> RECIP_D |
1056 ↗ | (On Diff #37847) | RINT -> RINT_D |
1059 ↗ | (On Diff #37847) | ROUND_L -> ROUND_L_D |
1063 ↗ | (On Diff #37847) | ROUND_W -> ROUND_W_D |
1066 ↗ | (On Diff #37847) | SEL -> SEL_D |
lib/Target/Mips/Mips32r6InstrInfo.td | ||
716–725 ↗ | (On Diff #37847) | Please move these back to where they were and use a separate 'let AdditionalPredicates ...'. They were in documentation order. |
lib/Target/Mips/MipsInstrFPU.td | ||
270–279 ↗ | (On Diff #37847) | Why have you removed these from 32R6/64R6? 32R6/64R6 didn't change these instructions Also, multiple ISA_*'s on the same instruction doesn't work since it overrides the same sub-list from PredicateControl. |
LGTM with a possible nit
lib/Target/Mips/MicroMips32r6InstrInfo.td | ||
---|---|---|
1166 ↗ | (On Diff #40277) | 80 cols? It looks long but I haven't counted. |