[mips] Support LA expansion in PIC mode

[sdardis]

Summary

Adds support for LA expansion in PIC mode.

Patch By: Srdjan Obucina

[dsanders]

The code change looks right to me but I have some comments about the test case.

Rather than make a new test file, could you extend test/MC/Mips/macro-la.s? This is mostly to keep our test directories tidy but it will also allow us to test that the other cases (e.g. 'la $5, 1') are unaffected and we'll have better coverage of the variety of expressions (e.g. 'la $5, symbol+8($6)', 'la $6, symbol+8($6)', and 'la $5, 1f').

[obucina]

We couldn't extend test/MC/Mips/macro-la.s because some tests in it failed in pic mode. For example, 'la $5, symbol+8' gives: 'LLVM ERROR: unsupported reloc value'. Also, istruction la in the case 'la $5, 1f' does not expand properly because symbol 1 is defined at the end of the test file, so condition 'Sym->isInSection()' fails because of one pass structure of assembler.

[obucina]

Ping

[mpf]

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

2485

The problem here is that you do not know whether a symbol is going to have global or local linkage until the end of the module as the .global directive may appear later. In order to do this you will need to track which symbols are referred to during macro expansion and then at the end of the file check that all the symbols still have the same linkage. If any have changed then raise an error. The restriction you will end up with in LLVM is that a global symbol must be globalized before the first reference to it.

The test cases are:

global

.text
foo:
la $4, bar

local

.data
bar:
.word 1
.text
foo:
la $4, bar

global

.data
.global bar
bar:
.word 1
.text
foo:
la $4, bar

local (but an error for LLVM)

.text
foo:
la $4, bar
.data
bar:
.word 1

global (but an error for LLVM)

.data
bar:
.word 1
.text
foo:
la $4, bar
.global bar

If the cases which work are sufficient for the codebases you need to support then this is relatively easy to implement. If you need the last two cases to work then there will be a significant amount of framework required as I understand.

[dsanders]

For example, 'la $5, symbol+8' gives: 'LLVM ERROR: unsupported reloc value'.

Whatever caused this seems to have been fixed. It emits this for me:

lw $6, %got(symbol)($gp)   # encoding: [A,A,0x86,0x8f]

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

2485

The problem here is that you do not know whether a symbol is going to have global or local
linkage until the end of the module as the .global directive may appear later.

I should have noticed that given that we've recently had the same problem with jal.

In order to do this you will need to track which symbols are referred to during macro
expansion and then at the end of the file check that all the symbols still have the same
linkage. If any have changed then raise an error. The restriction you will end up with in LLVM
is that a global symbol must be globalized before the first reference to it.

I like this solution. There's some valid inputs to GAS that won't be supported but it only takes a trivial source change (inserting a '.global bar') to work around it.

I'd limit the 'reference' part of your last sentence to just references involved in this kind of linkage-specific expansion since things like:

  .data
foo:
  .word bar
bar:
  .word 1

aren't a problem.

We only need one of the two error cases to be an error don't we? I think we could assume local and error out if it turned out to be global, or assume global and error out if it turned out to be local.

If you need the last two cases to work then there will be a significant amount of framework required as I understand.

That's my understanding too. We don't have a way to see the whole input before we have to make a decision on how to expand.

test/MC/Mips/la.pic.s

15

This instruction is incomplete

[mpf]

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

2485

We only need one of the two error cases to be an error don't we?

No I think both ways round will happen. All symbols will need to start off presumed to be global to match gas and for the case where they are not defined in the current module at all. If they are defined without .global before being referenced then they are local but could become global later. If they are not declared before being referenced they are global but may become local if defined later.

Only if we assumed local could we eliminate one of the error cases but that would lead to all externally defined globals needing an explicit .global in each file that references them... which is contrary to gas behaviour.

Not sure if that description is too technically dense. I'll try again if it is impenetrable.

[dsanders]

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

2485

I see what I've missed. The assumptions needed to make either of the latter two cases 'just work' directly contradict the assumptions needed to make the first three cases work.

The overall assumptions when processing the 'la $4, bar' are:

• If the symbol has never been seen then assume global
• If a .global/.local has been seen then it's known to be global/local
• If the definition has been seen but no .local/.global has been seen then assume it's local

[sdardis]

Commandeering this to repost.

[sdardis]

Update to trunk, account in tests for difference between emitting assembly and objects.

[dsanders]

Just removing a 3 year old patch from my queue. Feel free to re-add me if this is still needed and you want me to take another look

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

if (UseSrcReg &&
     // If $rs is the same as $rd, we need to use AT.
     // If it is not available we exit.
     unsigned ATReg = getATReg(IDLoc);
     if (!ATReg)
       return true;
     TmpReg = ATReg;
   }
 
-  emitRX(Mips::LUi, TmpReg, MCOperand::createExpr(HiExpr), IDLoc, Instructions);
+  const MCSymbol *Sym = getSingleMCSymbol(SymExpr);
+
+  if (inPicMode()) {
+    const MCExpr *Got16RelocExpr = evaluateRelocExpr(SymExpr, "got");
+    emitRRX(Mips::LW, TmpReg, Mips::GP, MCOperand::createExpr(Got16RelocExpr),
+            IDLoc, Instructions);
+    if(Sym->isInSection() && !Sym->isExternal())
+       emitRRX(Mips::ADDiu, TmpReg, TmpReg, MCOperand::createExpr(LoExpr),
+               IDLoc, Instructions);
+  } else {
+    emitRX(Mips::LUi, TmpReg, MCOperand::createExpr(HiExpr), IDLoc,
+           Instructions);
     emitRRX(Mips::ADDiu, TmpReg, TmpReg, MCOperand::createExpr(LoExpr), IDLoc,
             Instructions);
+  }
 
   if (UseSrcReg)
     emitRRR(Mips::ADDu, DstReg, TmpReg, SrcReg, IDLoc, Instructions);
   else
     assert(
         getContext().getRegisterInfo()->isSuperOrSubRegisterEq(DstReg, TmpReg));
 
   return false;

test/MC/Mips/la.pic.s

+# RUN: llvm-mc  %s -triple=mipsel-unknown-linux -mcpu=mips32 -show-encoding -relocation-model=pic | FileCheck %s -check-prefix=CHECK-PIC
+
+la $5, symbol         # CHECK-PIC: lw $5, %got(symbol)($gp)        # encoding: [A,A,0x85,0x8f]
+                      # CHECK-PIC:                                 # fixup A - offset: 0, value: symbol@GOT, kind: fixup_Mips_GOT_Local
+la $5, symbol($6)     # CHECK-PIC: lw $5, %got(symbol)($gp)        # encoding: [A,A,0x85,0x8f]
+                      # CHECK-PIC:                                 # fixup A - offset: 0, value: symbol@GOT, kind: fixup_Mips_GOT_Local
+                      # CHECK-PIC: addu    $5, $5, $6              # encoding: [0x21,0x28,0xa6,0x00]
+la $6, symbol($6)     # CHECK-PIC: lw      $1, %got(symbol)($gp)   # encoding: [A,A,0x81,0x8f]
+                      # CHECK-PIC:                                 # fixup A - offset: 0, value: symbol@GOT, kind: fixup_Mips_GOT_Local
+                      # CHECK-PIC: addu    $6, $1, $6              # encoding: [0x21,0x30,0x26,0x00]
+la $4, foo
+                      # CHECK-PIC: lw      $4, %got(foo)($gp)      # encoding: [A,A,0x84,0x8f]
+                      # CHECK-PIC:                                 # fixup A - offset: 0, value: foo@GOT, kind: fixup_Mips_GOT_Local
+dl:
+la $5,
+                      # CHECK-PIC: lw      $5, %got(dl)($gp)       # encoding: [A,A,0x85,0x8f]
+                      # CHECK-PIC:                                 # fixup A - offset: 0, value: dl@GOT, kind: fixup_Mips_GOT_Local
+                      # CHECK-PIC: addiu   $5, $5, %lo(dl)         # encoding: [A,A,0xa5,0x24]
+                      # CHECK-PIC:                                 # fixup A - offset: 0, value: %lo(dl), kind: fixup_Mips_LO16
+.globl  dg
+dg:
+la $5, dg
+                      # CHECK-PIC: lw      $5, %got(dg)($gp)       # encoding: [A,A,0x85,0x8f]
+                      # CHECK-PIC:                                 # fixup A - offset: 0, value: dg@GOT, kind: fixup_Mips_GOT_Local
+                      # CHECK-PIC: addiu   $5, $5, %lo(dg)         # encoding: [A,A,0xa5,0x24]
+                      # CHECK-PIC:                                 # fixup A - offset: 0, value: %lo(dg), kind: fixup_Mips_LO16
+
+
+