[PATCH] mips: add synci instruction

Daniel Sanders daniel.sanders at imgtec.com
Thu Nov 27 07:12:50 PST 2014


Thanks. LGTM with a couple formatting nits.

================
Comment at: lib/Target/Mips/Disassembler/MipsDisassembler.cpp:256-258
@@ +255,5 @@
+static DecodeStatus DecodeSyncI(MCInst &Inst,
+                              unsigned Insn,
+                              uint64_t Address,
+                              const void *Decoder);
+
----------------
Nit: Indentation

================
Comment at: lib/Target/Mips/MipsInstrFormats.td:415
@@ +414,3 @@
+class SYNCI_FM : StdArch {
+  bits<21> addr; // produced by the mem_simm16 address as reg << 16 | imm (see getMemEncoding)
+  bits<5> rs = addr{20-16};
----------------
Nits: Lines should be 80-columns, the first letter of the comment should be a capital P, and the comment should end with a '.'

================
Comment at: lib/Target/Mips/MipsInstrInfo.td:437
@@ +436,3 @@
+  let PredicateMethod = "isMemWithSimmOffset<16>";
+  //let DiagnosticType = "Simm16";
+}
----------------
I was going to say that commented out code should be deleted but then I saw that MipsMemSimm11AsmOperand above does the same thing. Up to you if you delete this line (and the one in MipsMemSimm11AsmOperand) in this patch or not. I'll delete them in a follow up if you choose not to.

http://reviews.llvm.org/D6421






More information about the llvm-commits mailing list