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

[Daniel Sanders]

> Regarding -mxgot, AFAICT it is never checked for in the IAS. There is a

>  command-line option used in llc, but I can't find a way to check for it from

>  MipsAsmParser. In the meantime, I've added a FIXME comment.


Ok. We'll have to add that at some point.

> Also, note that offsets do not work (e.g. jal label+8). I can get the value

>  of the offset, but I can't use it as an operand for the ADDIU, in the case

>  of O32, or store it in the relocation addendum, in the case of N32/N64.

>  I think we would need to improve relocation emission in order to properly handle offsets.


I believe the problem is that you are extracting the symbol from the expression, then creating a new expression from the symbol. The original expression should have the offset in it and will result in relocations for label+offset.


================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1331
@@ +1330,3 @@
+
+    if (JalSym.isInSection()) {
+      if (isABI_O32()) {
----------------
tomatabacu wrote:
> dsanders wrote:
> > I doubt this condition is right, although it's probably close enough to cover a lot of cases.
> > 
> > Things that spring to mind:
> > * Are forward-declared locals handled correctly?
> > * Are weak symbols handled correctly?
> > * What if the symbol is a section name like .text?
> Are forward-declared locals handled correctly?
> Unfortunately, they are not (I assume you're talking about doing a "jal fwd_label" before the definition of fwd_label). They end up being treated as global symbols. I'm not sure how to fix this.
> 
> Are weak symbols handled correctly?
> AFAICT weak symbols are treated the same way as global symbols.
> 
> What if the symbol is a section name like .text?
> .text should be treated as a local symbol, but there is something weird happening: with llvm-objdump it's treated as a local symbol, but with plain llvm-mc, .text is treated as a global symbol.
> Are forward-declared locals handled correctly?
> Unfortunately, they are not (I assume you're talking about doing a "jal fwd_label" before the definition of fwd_label). They end up being treated as global symbols. I'm not sure how to fix this.

Yes, I'm talking about cases like:
    jal foo
  foo:
I've just experimented with it and it turns out that they do work as expected and are treated as local symbols.

> Are weak symbols handled correctly?
> AFAICT weak symbols are treated the same way as global symbols.

Apart from a duplicate nop which needs fixing, this is behaving the same as GAS too.

> What if the symbol is a section name like .text?
> .text should be treated as a local symbol, but there is something weird happening: with llvm-objdump it's treated as a local symbol, but with plain llvm-mc, .text is treated as a global symbol.

I see this odd behaviour too. We need llvm-mc to do the right thing for testing purposes so could you look into this? My guess is that there's a difference in how the .text section is created.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1390-1403
@@ +1389,16 @@
+      }
+    } else {
+      // If it's an external/weak symbol, we expand to:
+      //  lw    $25, 0($gp)
+      //    R_MIPS_CALL16  label  /  R_MICROMIPS_CALL16  label
+      //  jalr  $25
+      MCInst LwInst;
+      LwInst.setOpcode(Mips::LW);
+      LwInst.addOperand(MCOperand::CreateReg(Mips::T9));
+      LwInst.addOperand(MCOperand::CreateReg(Mips::GP));
+      const MCSymbolRefExpr *Call16RelocSymExpr = MCSymbolRefExpr::Create(
+          JalSym.getName(), MCSymbolRefExpr::VK_Mips_GOT_CALL, getContext());
+      LwInst.addOperand(MCOperand::CreateExpr(Call16RelocSymExpr));
+      Instructions.push_back(LwInst);
+    }
+
----------------
tomatabacu wrote:
> dsanders wrote:
> > Is this doing the right thing for N64?
> Except for not generating LD on N64 (now fixed), I believe so.
Thanks. I've experimented a bit with GAS too and my remaining doubts are now settled. The relocations tend to change for N64 but this is one case where it doesn't.

http://reviews.llvm.org/D6231

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