[PATCH] D16917: [mips][micromips] Implement DCLO, DCLZ, DROTR, DROTR32 and DROTRV instructions

Daniel Sanders via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 7 02:09:59 PDT 2016


dsanders added inline comments.

================
Comment at: lib/Target/Mips/MicroMips64r6InstrInfo.td:250-251
@@ +249,4 @@
+class DSRA32_MM64R6_DESC : SHIFT_ROTATE_IMM_MM64R6<"dsra32", uimm5, II_DSRA32>;
+class DROTR_MM64R6_DESC : SHIFT_ROTATE_IMM_MM64R6<"drotr", uimm5, II_DROTR,
+                                                  rotr, immZExt5>;
+class DROTR32_MM64R6_DESC : SHIFT_ROTATE_IMM_MM64R6<"drotr32", uimm5,
----------------
sdardis wrote:
> Like DSLL_, DSRA_, DROTR_MM64R6_DESC should take a uimm6 & immZExt6.
> 
> You will then have to modify MipsMCCodeEmitter::encodeInstruction & LowerLargeShift as well to pick the correct D<op> or D<op>32.
An alternative way to do this is to make the DROTR32_*_DESC class use immZExtPlus32 so that SelectionDAG picks the correct instruction up front. Then the assembler part can be added using an appropriate InstAlias using the uimm5_plus32 operand. The main benefit of this approach is that we can leave the implementation to tablegen instead of writing custom C++.

I have a slight preference for the tablegen approach but if you go that route then you should also change the MIPS64 version for consistency. If you don't want to change the MIPS64 version right now then I'm ok with using the uimm6 and MipsMCCodeEmitter approach Simon describes for now.


http://reviews.llvm.org/D16917





More information about the llvm-commits mailing list