[PATCH] [mips] [IAS] Add support for the B{L, G}{T, E}(U) branch pseudo-instructions.

[Daniel Sanders]

I think there's a couple bugs in this. GAS and IAS behave differently for the following input:

  t1:
  blt $0, $0, t1
  blt $0, $2, t1
  blt $2, $0, t1
  blt $2, $2, t1
  blt $2, $3, t1
  bltu $0, $0, t1
  bltu $0, $2, t1
  bltu $2, $0, t1
  bltu $2, $2, t1
  bltu $2, $3, t1
  ble $0, $0, t1
  ble $0, $2, t1
  ble $2, $0, t1
  ble $2, $2, t1
  ble $2, $3, t1
  bleu $0, $0, t1
  bleu $0, $2, t1
  bleu $2, $0, t1
  bleu $2, $2, t1
  bleu $2, $3, t1
  bgt $0, $0, t1
  bgt $0, $2, t1
  bgt $2, $0, t1
  bgt $2, $2, t1
  bgt $2, $3, t1
  bgtu $0, $0, t1
  bgtu $0, $2, t1
  bgtu $2, $0, t1
  bgtu $2, $2, t1
  bgtu $2, $3, t1
  bge $0, $0, t1
  bge $0, $2, t1
  bge $2, $0, t1
  bge $2, $2, t1
  bge $2, $3, t1
  bgeu $0, $0, t1
  bgeu $0, $2, t1
  bgeu $2, $0, t1
  bgeu $2, $2, t1
  bgeu $2, $3, t1

Assembling with each assembler then dissassembling the output with objdump shows several differences including missing/additional nops, different offsets (mostly caused by the nop differences), and different instructions. There's also a '...' in the disassembly for the IAS case and I'm not sure what that indicates. In each case the IAS is functionally equivalent, and in some cases is arguably better (e.g. 'blez $0, $0, t1' -> 'b t1'). However, we ought to be producing the same opcodes as GAS given the same input.

<hr class="remarkup-hr" />

We should have a test that a warning is emitted when '.set noat' is in effect.


================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:196
@@ -195,1 +195,3 @@
 
+  bool expandCBranches(MCInst &Inst, SMLoc IDLoc,
+                       SmallVectorImpl<MCInst> &Instructions);
----------------
We should rename this to avoid ambiguity between conditional branches and compact branches

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1567-1577
@@ -1571,6 +1566,13 @@
 
-  if (needsExpansion(Inst))
-    return expandInstruction(Inst, IDLoc, Instructions);
-  else
+  if (needsExpansion(Inst)) {
+    if (expandInstruction(Inst, IDLoc, Instructions))
+      return true;
+  } else {
     Instructions.push_back(Inst);
+  }
+
+  // If this instruction has a delay slot and .set reorder is active,
+  // emit a NOP after it.
+  if (MCID.hasDelaySlot() && AssemblerOptions.back()->isReorder())
+    createNop(hasShortDelaySlot(Inst.getOpcode()), IDLoc, Instructions);
 
----------------
This should probably be split into a separate patch which you can commit (with the braces nit fixed) without further review.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1570-1572
@@ -1575,1 +1569,5 @@
+      return true;
+  } else {
     Instructions.push_back(Inst);
+  }
+
----------------
Nit: Added unnecessary braces.

================
Comment at: lib/Target/Mips/MipsInstrInfo.td:1667
@@ +1666,3 @@
+let hasDelaySlot = 1 in {
+class CBranchPseudo<string instr_asm> :
+  MipsAsmPseudoInst<(outs), (ins GPR32Opnd:$rs, GPR32Opnd:$rt,
----------------
We should rename this to avoid ambiguity between conditional branches and compact branches

http://reviews.llvm.org/D8537

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/