[PATCH] D16925: [mips] Support LA expansion in PIC mode

[Daniel Sanders via llvm-commits]

dsanders added inline comments.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:2485
@@ +2484,3 @@
+            IDLoc, Instructions);
+    if(Sym->isInSection() && !Sym->isExternal())
+       emitRRX(Mips::ADDiu, TmpReg, TmpReg, MCOperand::createExpr(LoExpr),
----------------
mpf wrote:
> dsanders wrote:
> > mpf wrote:
> > > 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.
> > > 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.
> > 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.
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


http://reviews.llvm.org/D16925