[PATCH] D16625: [mips][micromips] Implement LD, LLD, LWU, SD, DSRL, DSRL32 and DSRLV instructions

Simon Dardis via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 22 05:42:59 PDT 2016


sdardis added a subscriber: sdardis.
sdardis requested changes to this revision.
sdardis added a reviewer: sdardis.
sdardis added a comment.
This revision now requires changes to proceed.

I notice that you've implemented LLD here but I am not seeing any outstanding patches for SCD. Is this an oversight or is there another patch to be submitted to implement that?


================
Comment at: lib/Target/Mips/MicroMips64r6InstrFormats.td:88
@@ +87,3 @@
+
+class LD_SD_32_2ROFFSET16_FM_MMR6<string instr_asm, bits<6> op>
+    : MMR6Arch<instr_asm> {
----------------
Can you put a _ between 2R and OFFSET16? Makes it easier to read.

================
Comment at: lib/Target/Mips/MicroMips64r6InstrFormats.td:103
@@ +102,3 @@
+
+class POOL32C_2ROFFSET12_FM_MMR6<string instr_asm, bits<4> funct>
+    : MMR6Arch<instr_asm> {
----------------
Here too.

================
Comment at: lib/Target/Mips/MicroMips64r6InstrFormats.td:119
@@ +118,3 @@
+
+class POOL32S_2RSA5_FM_MMR6<string instr_asm, bits<9> funct> {
+  bits<5> rt;
----------------
Like my previous comments, and my comment on this class in D16917, can you put a '_' between 2R and SA5?

================
Comment at: lib/Target/Mips/Mips64InstrInfo.td:186
@@ -179,2 +185,3 @@
 
 def LWu   : Load<"lwu", GPR64Opnd, zextloadi32, II_LWU>, LW_FM<0x27>, ISA_MIPS3;
+let AdditionalPredicates = [NotInMicroMips] in {
----------------
Shouldn't this also be covered by NotInMicroMips?

================
Comment at: test/MC/Mips/micromips64r6/invalid.s:146-149
@@ -145,1 +145,5 @@
   swm16 $16, $17, $ra, 64($sp) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
+  ld $32, 65536($32)           # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
+  lld $32, 4096($32)           # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
+  lwu $32, 4096($32)           # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
+  sd $32, 65536($32)           # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
----------------
Can you add some out of range negative values too?

================
Comment at: test/MC/Mips/mips64r6/invalid.s:51
@@ +50,2 @@
+        dsrl32 $32, $32, 32  # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
+        dsrlv $32, $32, $32  # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
----------------
Can you add a negative immediate test case here for dsrl?


http://reviews.llvm.org/D16625





More information about the llvm-commits mailing list