HA: [PATCH] D15480: [ELF][MIPS] Handle R_MIPS_HI16/LO16 relocations against _gp_disp symbol

[Simon Atanasyan via llvm-commits]

In that case we even do not need to pass `A` because addend should be read
from the relication location. But we have to pass `nullptr` instead of
`Body` in many all cases of the `relicateOne` invocation. That is why I
rejected this solution when implement the patch. But if it looks better
than current variant, I am ready to change it.
On Dec 24, 2015 3:39 PM, "George Rimar" <grimar at accesssoftek.com> wrote:

> >On Thu, Dec 24, 2015 at 2:22 PM, George Rimar <grimar at accesssoftek.com>
> wrote:
> >>>>I thinking about that place last time.
> >>>>May be we should add A argument to Target->relocateOne()  ?
> >>>>That can help to move that calculation to mips target, since it looks
> like absence of A inside relocateOne() is the only stoppable for doing that.
> >>>>
> >>>Currently S+A is a single parameter SA. Are you suggesting split that
> as two parameters, S and A?
> >>>
> >>>I don't know if that makes code simpler, but it may worth trying.
> >>
> >> Not sure about splitting. That seems to be more complicated. Just
> adding A can save from complicating logic outside allowing relocateOne to
> use it if it is needed for concrete relocation`s code like here for mips.
> >
> >What part of the `getMipsGpAddr<ELFT>() - AddrLoc` expression do you
> >suggest to pass a a separate A argument? And do you suggest to pass
> >the `Body` to the `relocateOne` as well to be able to recognize a
> >special _gp_disp case of R_MIPS_HI16 / LO16 relocations?
> >
> >--
> >Simon Atanasyan
>
> Whole calculations can be inside relocateOne().
> Yes, forgot about body, its also needed. So it could probably look
> something like that:
>
> template <class ELFT>
> void MipsTargetInfo<ELFT>::relocateOne(uint8_t *Loc, uint8_t *BufEnd,
>                                        uint32_t Type, uint64_t P, uint64_t
> SA,
>                                        uint64_t ZA, uint8_t *PairedLoc,
>                                        uint64_t A, const SymbolBody *S)
> const {
>   const endianness E = ELFT::TargetEndianness;
>   switch (Type) {
> ....
>   case R_MIPS_HI16: {
>     if (S == Config->MipsGpDisp)
>       SA = getMipsGpAddr<ELFT>() - P + A;
>     uint32_t Instr = read32<E>(Loc);
>     if (PairedLoc) {
>       uint64_t AHL = ((Instr & 0xffff) << 16) +
>                      SignExtend64<16>(read32<E>(PairedLoc) & 0xffff);
>       write32<E>(Loc, (Instr & 0xffff0000) | mipsHigh(SA + AHL));
>     } else {
>       warning("Can't find matching R_MIPS_LO16 relocation for
> R_MIPS_HI16");
>       write32<E>(Loc, (Instr & 0xffff0000) | mipsHigh(SA));
>     }
>     break;
>   }
> ......
>   case R_MIPS_LO16: {
>     if (S == Config->MipsGpDisp)
>       SA = getMipsGpAddr<ELFT>() - P + A + 4;
>     uint32_t Instr = read32<E>(Loc);
>     int64_t AHL = SignExtend64<16>(Instr & 0xffff);
>     write32<E>(Loc, (Instr & 0xffff0000) | ((SA + AHL) & 0xffff));
>     break;
>   }
> ......
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151224/1b12170f/attachment.html>