[PATCH] [mips] Expand JAL instructions when PIC is enabled.

[Toma Tabacu]

In http://reviews.llvm.org/D6231#191632, @dsanders wrote:

> That's ok for this patch, but following the principle above I'd suggest asserting that LargeGOT is not in use.


I just realized that llvm-mc doesn't even accept -mxgot on the command line (it's also not listed in --help-hidden).
It seems that -mxgot is a hidden option only for llc.

> Didn't we establish earlier in the review that these worked as expected? Has that changed?


I'm not sure we did. We're still treating forward-declared local symbols as global, while GAS treats them as local.
Just to be clear, I'm talking the following:

  .option pic2
    jal foo
  foo:


================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1273-1274
@@ +1272,4 @@
+static const MCSymbol *getSingleMCSymbol(const MCExpr *Expr) {
+  if (const MCSymbolRefExpr *SRExpr = dyn_cast<MCSymbolRefExpr>(Expr)) {
+    return &(SRExpr->getSymbol());
+  }
----------------
dsanders wrote:
> Nit: Unnecessary braces
Fixed.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1461
@@ +1460,3 @@
+  // This expansion is not in a function called by expandInstruction() because
+  // the psudo-instruction doesn't have a distinct opcode.
+  if ((Inst.getOpcode() == Mips::JAL || Inst.getOpcode() == Mips::JAL_MM) &&
----------------
dsanders wrote:
> Nit: psudo->pseudo
Fixed.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1468-1473
@@ +1467,8 @@
+
+    // We can do this expansion if there's only 1 symbol in the argument
+    // expression.
+    if (countMCSymbolRefExpr(JalExpr) > 1)
+      return Error(IDLoc, "jal doesn't support multiple symbols in PIC mode");
+
+    const MCSymbol *JalSym = getSingleMCSymbol(JalExpr);
+
----------------
dsanders wrote:
> I'm not sure I understand this bit. Can you explain?
GAS spews all kinds of errors if it is given 2 symbols here, so we should report an error as well.
Also, if we didn't, the IAS would (currently) segfault.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1511
@@ +1510,3 @@
+        MCInst LoadInst;
+        LoadInst.setOpcode(isABI_N64() ? Mips::LD : Mips::LW);
+        LoadInst.addOperand(MCOperand::createReg(Mips::T9));
----------------
dsanders wrote:
> Nit: isABI_N64() -> ABI.ArePtrs64bit()
Fixed.

http://reviews.llvm.org/D6231

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