[PATCH] Re-enable a hook in MCELFObjectTargetWriter to allow target-specific relocationtable sorting and use this hook for Mips.
Vladimir Stefanovic
Vladimir.Stefanovic at imgtec.com
Tue Mar 10 08:17:39 PDT 2015
A new version of the patch is uploaded.
________________________________________
From: Rafael Ávila de Espíndola [rafael.espindola at gmail.com]
Sent: Thursday, February 19, 2015 7:03 PM
To: Vladimir Stefanovic; Daniel Sanders; simon at atanasyan.com
Cc: Toma Tabacu; Petar Jovanovic; llvm-commits at cs.uiuc.edu
Subject: Re: [PATCH] Re-enable a hook in MCELFObjectTargetWriter to allow target-specific relocationtable sorting and use this hook for Mips.
> 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 :-)
Well, you can have this in a .S file:
lui $2,%hi(sym1)
.set mips16
li $2,%lo(sym1)
Then relocations would be:
00000000 00000605 R_MIPS_HI16 00000000 sym1
00000004 00000669 R_MIPS16_LO16 00000000 sym1
so the first relocation wouldn't match the second.
> 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?
'match' for a *HI16 relocation is a matching LO16 relocation, ie. the one
against the same symbol and with the appropriate *LO16 relocation type.
'earlier' means smaller offset.
'in order' is similar to our cmpRel() sorting function - apply the first rule,
and if the output isn't deterministic yet, apply the second one, etc.
REPOSITORY
rL LLVM
http://reviews.llvm.org/D7414
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list