[PATCH] D13929: [mips][microMIPS] Implement SHLL.PH, SHLL_S.PH, SHLL.QB, SHLLV.PH, SHLLV_S.PH, SHLLV.QB, SHLLV_S.W, SHLL_S.W, SHRA.QB and SHRA_R.QB instructions

[Daniel Sanders via llvm-commits]

dsanders accepted this revision.
dsanders added a comment.
This revision is now accepted and ready to land.

LGTM with a couple nits.


================
Comment at: lib/Target/Mips/MicroMipsDSPInstrInfo.td:76
@@ -65,1 +75,3 @@
 
+class SHLL_QB_R2_MM_DESC_BASE<string instr_asm, SDPatternOperator OpNode,
+                              SDPatternOperator ImmPat, InstrItinClass itin,
----------------
Could you drop the '_QB'? This base class is used by the other SHLL's too.

================
Comment at: lib/Target/Mips/MicroMipsDSPInstrInfo.td:80
@@ +79,3 @@
+  dag OutOperandList = (outs RO:$rt);
+  dag InOperandList = (ins RO:$rs, uimm16:$sa);
+  string AsmString = !strconcat(instr_asm, "\t$rt, $rs, $sa");
----------------
This uimm16 should be uimm3, uimm4, or uimm5 depending on the exact instruction. There's no functional difference right now but I'm starting to fix the missing range checks.

If any of these don't exist then add them alongside the other uimm operand definitions.

================
Comment at: lib/Target/Mips/MicroMipsDSPInstrInfo.td:101
@@ +100,3 @@
+
+class SHLL_QB_R3_MM_DESC_BASE<string instr_asm, SDPatternOperator OpNode,
+                              InstrItinClass itin, RegisterOperand RO> {
----------------
Could you clarify that this is SHLLV and not SHLL? I was confused by the lack of an immediate in operand list at first.

Also, could you drop the '_QB'? This base class is used by the other SHLLV's too.


http://reviews.llvm.org/D13929