[PATCH] Re-enable a hook in MCELFObjectTargetWriter to allow target-specific relocationtable sorting and use this hook for Mips.

[Rafael Ávila de Espíndola]

> About the relocations that must match another:

> 

> -mips32, mips64:

>  R_MIPS_HI16 and local R_MIPS_GOT16 must match R_MIPS_LO16 against the same 

>  symbol.

> 

> -micromips:

>  R_MICROMIPS_HI16 and local R_MICROMIPS_GOT16 must match R_MICROMIPS_LO16 

>  against the same symbol.

> 

> -mips16:

>  R_MIPS16_HI16 and local R_MIPS16_GOT16 must match R_MIPS16_LO16 against the same 

>  symbol.


And it is not possible for a file to mix relocations from these sets? I.E, no file will ever have a R_MIPS16_LO16 and a R_MIPS_HI16? If that is not the case I think there is a bug in gold :-)

> Now, since mips16 and micromips are a work in progress, I would rather skip 

>  handling them for now (ie. return generic sortRelocs() and add a TODO for the 

>  HI16/GOT16 exceptions). E.g., instead of R_MICROMIPS_LO16, llvm currently 

>  generates R_MIPS_LO16, so I can't even add a micromips test that will pass at 

>  the moment.


That is fine. An assert would be better than a comment to make sure it gets implemented :-)

> For the deterministic output - in the examples I ran, when entering sortRelocs() 

>  relocs were sorted by offset in the ascending order. But I will add a call to 

>  generic sortRelocs() at the beginning of the function, to make the output 

>  deterministic for sure.


Yes, I honestly cannot remember where the non determinism was coming from, but I do remember the original function sorting only by Offest and we having issues.

> And, apart from HI16/GOT16, I would like to sort relocs by offset - like other 

>  architectures and mips gcc do.


MC suffer a lot from trying to produce object files that look like what gas produces. I know this is mostly my fault, but if I can help us move to a point where every 'if' in the writer has a good justification that would be awesome.

> A quote from binutils source (binutils/bfd/elfxx-mips.c):

> 

>   The ABI requires that the *LO16 immediately follow the *HI16.

>   However, as a GNU extension, we permit an arbitrary number of

>   *HI16s to be associated with a single *LO16.


Check. It seems to be what gold implements.

> The logic I used in the code below is the simplest I came up with to obey the 

>  rule above: for every HI16 / local GOT16 relocation at the given offset, pair it 

>  with the first found LO16 relocation against the same symbol, starting from 

>  offset + 4 and ending at offset -4. (Wrap around reloc table size.)


That is something that needs to be explicitly documented, since it is the semantic of the of assembly file, which is potentially an user input.

> GCC does it differently; here is a comment about it from 

>  binutils/gas/config/tc-mips.c:

> 

>   When several %lo()s match a particular %got() or %hi(), we use the

>   following rules to distinguish them:

>    

>     (1) %lo()s with smaller offsets are a better match than %lo()s with

>         higher offsets.

>    

>     (2) %lo()s with no matching %got() or %hi() are better than those

>         that already have a matching %got() or %hi().

>    

>     (3) later %lo()s are better than earlier %lo()s.

>   These rules are applied in order.

>    

>    

> 

> Thus, for this example:

> 

>   lui    $2, %hi(func2)

>   lui    $2, %hi(func2)

>   addiu  $2, $2, %lo(func2)

>   addiu  $2, $2, %lo(func2)

>    

>    

> 

> the code below sorts the table like this:

> 

>   Offset     Info    Type            Sym.Value  Sym. Name

> 

> 00000000  00000605 R_MIPS_HI16       00000000   func2

>  00000004  00000605 R_MIPS_HI16       00000000   func2

>  00000008  00000606 R_MIPS_LO16       00000000   func2

>  0000000c  00000606 R_MIPS_LO16       00000000   func2

> 

> and this is what gcc does:

> 

>   Offset     Info    Type            Sym.Value  Sym. Name

> 

> 00000004  00000605 R_MIPS_HI16       00000000   func2

>  00000008  00000606 R_MIPS_LO16       00000000   func2

>  00000000  00000605 R_MIPS_HI16       00000000   func2

>  0000000c  00000606 R_MIPS_LO16       00000000   func2

> 

> So, at least for consistency reasons, maybe I should change this code to behave 

>  like gcc. What do you think?


I agree. We should implement the same *semantics* as gas.

In the above comment, what does "match", "earlier" and "in order" mean?


REPOSITORY
  rL LLVM

http://reviews.llvm.org/D7414

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