[PATCH] D14206: [mips][microMIPS] Implement BGEC, BGEUC, BLTC, BLTUC, BEQC and BNEC instructions

[Daniel Sanders via llvm-commits]

dsanders accepted this revision.
dsanders added a comment.
This revision is now accepted and ready to land.

LGTM with a few nits.

The RDHWR/RDGPR/LUI changes should be in a separate patch


================
Comment at: lib/Target/Mips/Disassembler/MipsDisassembler.cpp:2083-2168
@@ -2072,1 +2082,87 @@
 }
+
+template <typename InsnType>
+static DecodeStatus DecodeBgtzGroupBranchMMR6(MCInst &MI, InsnType insn,
+  uint64_t Address,
+  const void *Decoder) {
+  // We have:
+  //    0b000111 ttttt sssss iiiiiiiiiiiiiiii
+  //      Invalid      if rt == 0
+  //      BGTZALC_MMR6 if rs == 0 && rt != 0
+  //      BLTZALC_MMR6 if rs != 0 && rs == rt
+  //      BLTUC_MMR6   if rs != 0 && rs != rt
+
+  InsnType Rt = fieldFromInstruction(insn, 21, 5);
+  InsnType Rs = fieldFromInstruction(insn, 16, 5);
+  InsnType Imm = SignExtend64(fieldFromInstruction(insn, 0, 16), 16) * 2;
+  bool HasRs = false;
+  bool HasRt = false;
+
+  if (Rt == 0) {
+    return MCDisassembler::Fail;
+  }
+  else if (Rs == 0) {
+    MI.setOpcode(Mips::BGTZALC_MMR6);
+    HasRt = true;
+  }
+  else if (Rs == Rt) {
+    MI.setOpcode(Mips::BLTZALC_MMR6);
+    HasRs = true;
+  }
+  else {
+    MI.setOpcode(Mips::BLTUC_MMR6);
+    HasRs = true;
+    HasRt = true;
+  }
+
+  if (HasRs)
+    MI.addOperand(MCOperand::createReg(getReg(Decoder, Mips::GPR32RegClassID,
+    Rs)));
+
+  if (HasRt)
+    MI.addOperand(MCOperand::createReg(getReg(Decoder, Mips::GPR32RegClassID,
+    Rt)));
+
+  MI.addOperand(MCOperand::createImm(Imm));
+
+  return MCDisassembler::Success;
+}
+
+
+template <typename InsnType>
+static DecodeStatus DecodeBlezGroupBranchMMR6(MCInst &MI, InsnType insn,
+  uint64_t Address,
+  const void *Decoder) {
+  // We have:
+  //    0b000110 ttttt sssss iiiiiiiiiiiiiiii
+  //      Invalid        if rs == 0
+  //      BLEZALC_MMR6   if rs == 0  && rt != 0
+  //      BGEZALC_MMR6   if rs == rt && rt != 0
+  //      BGEUC_MMR6     if rs != rt && rs != 0  && rt != 0
+
+  InsnType Rt = fieldFromInstruction(insn, 21, 5);
+  InsnType Rs = fieldFromInstruction(insn, 16, 5);
+  InsnType Imm = SignExtend64(fieldFromInstruction(insn, 0, 16), 16) * 2;
+  bool HasRs = false;
+
+  if (Rt == 0)
+    return MCDisassembler::Fail;
+  else if (Rs == 0)
+    MI.setOpcode(Mips::BLEZALC_MMR6);
+  else if (Rs == Rt)
+    MI.setOpcode(Mips::BGEZALC_MMR6);
+  else {
+    HasRs = true;
+    MI.setOpcode(Mips::BGEUC_MMR6);
+  }
+
+  if (HasRs)
+    MI.addOperand(MCOperand::createReg(getReg(Decoder, Mips::GPR32RegClassID,
+    Rs)));
+  MI.addOperand(MCOperand::createReg(getReg(Decoder, Mips::GPR32RegClassID,
+    Rt)));
+
+  MI.addOperand(MCOperand::createImm(Imm));
+
+  return MCDisassembler::Success;
+}
----------------
Please clang-format this

================
Comment at: lib/Target/Mips/Disassembler/MipsDisassembler.cpp:2101-2103
@@ +2100,5 @@
+
+  if (Rt == 0) {
+    return MCDisassembler::Fail;
+  }
+  else if (Rs == 0) {
----------------
Redundant braces

================
Comment at: lib/Target/Mips/MicroMips32r6InstrFormats.td:20-24
@@ +19,7 @@
+
+//===----------------------------------------------------------------------===//
+//
+// Disambiguators
+//
+//===----------------------------------------------------------------------===//
+//
----------------
Could you also add the 'Encoding Formats' one before the encoding formats begin?

Also, MicroMipsR6Inst16 should probably be outside this section.

================
Comment at: lib/Target/Mips/MicroMips32r6InstrFormats.td:575
@@ -559,3 +574,3 @@
 class POOL32F_ARITH_FM_MMR6<string instr_asm, bits<2> fmt, bits<8> funct>
-    : MMR6Arch<instr_asm>, MipsR6Inst {
+    : MipsR6Inst, MMR6Arch<instr_asm> {
   bits<5> ft;
----------------
The 'MMR6Arch<instr_asm>, MipsR6Inst' -> 'MipsR6Inst, MMR6Arch<instr_asm>' tidy up ought to be in a separate patch which can be committed without further review.


http://reviews.llvm.org/D14206