[PATCH] D17334: [mips][microMIPS] Implement MFC*, MFHC* and DMFC* instructions

Daniel Sanders via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 29 03:12:10 PDT 2016


dsanders added a comment.

LGTM with a nit and missing predicate added.

> Added decoder table for 64-bit FPU version.


This hadn't occurred to me at the time but it raises a good point. The existing FP64 disassembly support is currently in the Mips64 decoder table but this means that disassembling Mips32+FP64 objects won't yield the correct MCInstr objects. Our existing disassembler tests won't detect this because the instructions are syntactically identical at the assembly level. This existing bug doesn't need fixing in this patch, I just mention it as something we'll need to address at some point.


================
Comment at: lib/Target/Mips/Disassembler/MipsDisassembler.cpp:922
@@ +921,3 @@
+
+    if (hasMips32r6()) {
+      DEBUG(dbgs() << "Trying MicroMips32r6FPU table (32-bit opcodes):\n");
----------------
You need to check for FP64 too.

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:1055
@@ +1054,3 @@
+                     ISA_MICROMIPS32R6;
+let DecoderNamespace = "MicroMips32r6FPU" in {
+  def MFHC1_D64_MMR6 : StdMMR6Rel, MFHC1_MMR6_ENC, MFHC1_D64_MMR6_DESC,
----------------
The 'FPU' suffix by itself is a little misleading. It's really about 64-bit FPU's (FP64) rather than FPU's in general.


http://reviews.llvm.org/D17334





More information about the llvm-commits mailing list