[PATCH] D11181: [mips][microMIPS] Implement BC16, BEQZC16 and BNEZC16 instructions

Daniel Sanders via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 18 03:51:24 PDT 2015


dsanders requested changes to this revision.
This revision now requires changes to proceed.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1705-1708
@@ -1701,4 +1704,6 @@
     return expandLoadAddressReg(Inst, true, IDLoc, Instructions);
   case Mips::B_MM_Pseudo:
     return expandUncondBranchMMPseudo(Inst, IDLoc, Instructions);
+  case Mips::B_MMR6_Pseudo:
+    return expandUncondBranchMMPseudo(Inst, IDLoc, Instructions);
   case Mips::SWM_MM:
----------------
Nit: These two cases are identical. Put both case-statements in front of one copy of the code.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:2131-2134
@@ -2125,3 +2130,6 @@
       // 16-bit unconditional branch instruction.
-      Inst.setOpcode(Mips::B16_MM);
+      if (inMicroMipsMode() && (hasMips32r6() || hasMips64r6()))
+        Inst.setOpcode(Mips::BC16_MMR6);
+      else
+        Inst.setOpcode(Mips::B16_MM);
     } else {
----------------
'|| hasMips64r6()' is redundant since the predicates are cumulative.

Also use the ternary operator:
  if (inMicroMipsMode())
    Inst.setOpcode(hasMips32r6() ? Mips::BC16_MMR6 : Mips::BC16_MM)

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:2150
@@ -2141,3 +2149,3 @@
   // If .set reorder is active, emit a NOP after the branch instruction.
-  if (AssemblerOptions.back()->isReorder())
+  if (AssemblerOptions.back()->isReorder() && Inst.getOpcode() != Mips::BC16_MMR6)
     createNop(true, IDLoc, Instructions);
----------------
This does the right thing but we shouldn't list all the compact branches explicitly. Test whether the instruction has a delay slot instead.

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:151
@@ +150,3 @@
+  let hasDelaySlot = 0;
+  let Predicates = [RelocPIC, HasStdEnc, HasMicroMips32r6];
+  let Defs = [AT];
----------------
Don't use Predicates directly, it causes lots of trouble that is hard to debug. People try to add a predicate and end up accidentally removing some. For example, these instructions lack the predicates that ISA_MICROMIPS32R6 is supposed to add.

Inherit from PredicateControl (you already do this since you inherit from MipsR6Inst) and set each mutually exclusive group independently.

Likewise for the other occasions you override Predicates.

================
Comment at: lib/Target/Mips/MicroMipsInstrInfo.td:931
@@ -930,2 +930,3 @@
 
-  def B_MM_Pseudo : UncondBranchMMPseudo<"b">;
+let Predicates = [InMicroMips] in {
+def B_MM_Pseudo : UncondBranchMMPseudo<"b">;
----------------
As above, don't override Predicates. Use PredicateControl's members or preferably, add a ISA_MICROMIPS and use that instead.


http://reviews.llvm.org/D11181





More information about the llvm-commits mailing list