[PATCH] D15418: [mips][microMIPS] Implement LH, LHE, LHU and LHUE instructions

[Simon Dardis via llvm-commits]

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

Hi,

After applying this patch and checking lh_lhu.ll with "-asm-show-inst" I see:

  lh      $2, %lo(us)($2)         # <MCInst #1187 LH
                                  #  <MCOperand Reg:321>
                                  #  <MCOperand Reg:321>
                                  #  <MCOperand Expr:(us at ABS_LO)>>

Shouldn't LLVM have picked LH_MM? The rest are small nits regarding some style issues and the ordering of instructions in test files.


================
Comment at: lib/Target/Mips/MicroMips32r6InstrFormats.td:883
@@ +882,3 @@
+  let Inst{20-16} = addr{20-16};
+  let Inst{15-0}  = addr{15-0};
+}
----------------
Can you redo this definition in the style of SB32_SH32_STORE_FM_MMR6 ? You split addr into base  and offset, then assign to Inst.

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:173-174
@@ -172,2 +172,4 @@
 class TLBINVF_MMR6_ENC : POOL32A_TLBINV_FM_MMR6<"tlbinvf", 0x14d>;
+class LH_MMR6_ENC : LH32_LHU32_FM_MMR6<"lh", 0xf>;
+class LHu_MMR6_ENC : LH32_LHU32_FM_MMR6<"lhu", 0xd>;
 
----------------
Can you use the 0bxxx style of specifying opcodes here? It makes it somewhat easier to check and is more consistent with the overall MIPS backend.

================
Comment at: test/MC/Disassembler/Mips/micromips32r6/valid.txt:261-264
@@ -260,1 +260,5 @@
 0x00 0x00 0x53 0x7c # CHECK: tlbinvf
+0x3c 0x44 0x00 0x08 # CHECK: lh $2, 8($4)
+0x60 0x82 0x6a 0x08 # CHECK: lhe $4, 8($2)
+0x34 0x82 0x00 0x08 # CHECK: lhu $4, 8($2)
+0x60 0x82 0x62 0x08 # CHECK: lhue $4, 8($2)
----------------
Can you sort these alphabetically into the existing instructions?

================
Comment at: test/MC/Mips/micromips-invalid.s:105
@@ +104,2 @@
+  lhu $4, 65536($2) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
+  lhue $4, 512($2)  # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
----------------
Can you add some out of range negative tests for these instructions as well? This applies to the other invalid tests as well.

================
Comment at: test/MC/Mips/micromips32r6/valid.s:255-258
@@ -254,1 +254,5 @@
   tlbinvf                  # CHECK: tlbinvf                # encoding: [0x00,0x00,0x53,0x7c]
+  lh $2, 8($4)             # CHECK: lh $2, 8($4)        # encoding: [0x3c,0x44,0x00,0x08]
+  lhe $4, 8($2)            # CHECK: lhe $4, 8($2)       # encoding: [0x60,0x82,0x6a,0x08]
+  lhu $4, 8($2)            # CHECK: lhu $4, 8($2)       # encoding: [0x34,0x82,0x00,0x08]
+  lhue $4, 8($2)           # CHECK: lhue $4, 8($2)      # encoding: [0x60,0x82,0x62,0x08]
----------------
Can you sort these alphabetically into the first 100~ instructions.


http://reviews.llvm.org/D15418