[PATCH] D18824: [mips][microMIPS] Implement LDC1, SDC1, LDC2, SDC2, LWC1, SWC1, LWC2 and SWC2 instructions and add CodeGen support

Simon Dardis via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 4 03:36:52 PDT 2016


sdardis accepted this revision.
sdardis added a comment.

LGTM with some nits (inlined).

You'll need to add -target-abi n64 for any tests that are targeting micromips64r6, as some of the defaults have changed.


================
Comment at: lib/Target/Mips/Mips32r6InstrInfo.td:764
@@ -764,1 +763,3 @@
+  def LDC2_R6 : LDC2_R6_ENC, LDC2_R6_DESC, ISA_MIPS32R6;
+  def LWC2_R6 : LWC2_R6_ENC, LWC2_R6_DESC, ISA_MIPS32R6;
   def LL_R6 : LL_R6_ENC, LL_R6_DESC, ISA_MIPS32R6;
----------------
Rather than moving this line up, can you keep the existing ordering and wrap that line in a NotInMicroMips predicate?

================
Comment at: test/CodeGen/Mips/mno-ldc1-sdc1.ll:10
@@ +9,3 @@
+; RUN:   -relocation-model=pic < %s | \
+; RUN:   FileCheck %s -check-prefix=ALL -check-prefix=MM
+; RUN: llc -march=mipsel -mcpu=mips32r6 -mattr=+micromips \
----------------
Since this patch was first posted, FileCheck gained a new option -check-prefixes=. See rL273669 on how to use it.

================
Comment at: test/MC/Disassembler/Mips/micromips32r3/valid-el.txt:194
@@ -193,1 +193,3 @@
 0x8c 0x01 0x7c 0x8b # CHECK: syscall 396
+0xea 0xbc 0x2c 0x01 # CHECK: ldc1 $f7, 300($10)
+0x0a 0xbd 0x2c 0x01 # CHECK: ldc1 $f8, 300($10)
----------------
You can eliminate the near duplicates of what you're adding unless the encoding is some how irregular. i.e. the register field is split into two parts. This goes for the other disassembler tests you're adding.

================
Comment at: test/MC/Mips/mips32r6/invalid.s:115
@@ +114,2 @@
+        lwc2 $32, 16($4)     # CHECK: :[[@LINE]]:14: error: invalid operand for instruction
+        swc2 $32, 777($17)   # CHECK: :[[@LINE]]:14: error: invalid operand for instruction
----------------
This test file also requires [ls][wd]dc2/ out of range tests.

================
Comment at: test/MC/Mips/mips64r6/invalid.s:107
@@ +106,2 @@
+        lwc2 $32, 16($4)     # CHECK: :[[@LINE]]:14: error: invalid operand for instruction
+        swc2 $32, 777($17)   # CHECK: :[[@LINE]]:14: error: invalid operand for instruction
----------------
This test file also requires [ls][wd]dc2/ out of range tests.


http://reviews.llvm.org/D18824





More information about the llvm-commits mailing list