This is an archive of the discontinued LLVM Phabricator instance.

[ELF][MIPS] Make _gp, _gp_disp, __gnu_local_gp global symbols
ClosedPublic

Authored by atanasyan on Dec 7 2016, 8:07 AM.

Details

Summary

These MIPS specific symbols should be global because in general they can have an arbitrary value. By default this value is a fixed offset from .got section.

This patch adds more checks to the mips-gp-local.s test case but marks it as XFAIL because LLD does not allow redefinition of absolute symbols value by a linker script. This should be fixed by D27276.

Diff Detail

Repository
rL LLVM

Event Timeline

atanasyan updated this revision to Diff 80598.Dec 7 2016, 8:07 AM
atanasyan retitled this revision from to [ELF][MIPS] Make _gp, _gp_disp, __gnu_local_gp global symbols.
atanasyan updated this object.
atanasyan added a reviewer: ruiu.
atanasyan set the repository for this revision to rL LLVM.
atanasyan added a project: lld.
atanasyan added subscribers: llvm-commits, meadori.
ruiu edited edge metadata.Dec 7 2016, 11:13 AM

I wonder if

ELF/Relocations.cpp
351–353 ↗(On Diff #80598)

Doesn't _gp_disp represents offset between _gp and .got? I'm not sure what you mean by "a function" in the above description.

Other than the one question above, LGTM. I tested this patch with my WIP patch for D27276. Thank you for fixing this.

ELF/Relocations.cpp
357 ↗(On Diff #80598)

Is this related to the problem being discussed in D26133?

atanasyan added inline comments.Dec 7 2016, 12:44 PM
ELF/Relocations.cpp
351–353 ↗(On Diff #80598)

Doesn't _gp_disp represents offset between _gp and .got? I'm not sure what you mean by "a function" in the above description.

By definition _gp_disp represents the offset between the beginning of the function and the _gp. This symbol is used in a position-independent function prologue in a combination with R_MIPS_HI16/LO16 relocations. So for each function _gp_disp has a unique value. Moreover we do not have to really define this symbol and put it to the symbol table. From the other side we need to define this symbol to escape "not defined symbol" error.

Your question gives me a good idea. Maybe it is better to add a new expression R_MIPS_GP_DISP for this case. So we will not mix these specific relocations with regular R_PC relocations and will be able to write checking more clear.

357 ↗(On Diff #80598)

Is this related to the problem being discussed in D26133?

Yes and no. With D26133 we will not need this if condition. From the other side _gp_disp is not true absolute symbol but just a "marker". If we could suppress "symbol not found error" for _gp_disp we even would not need to define it, add to a symbol table etc.

Doesn't _gp_disp represents offset between _gp and .got? I'm not sure what you mean by "a function" in the above description.

Your question gives me a good idea. Maybe it is better to add a new expression R_MIPS_GP_DISP for this case. So we will not mix these specific relocations with regular R_PC relocations and will be able to write checking more clear.

Not so good as expected. The code is not reduced and we have to put new R_MIPS_GP_DISP expression type in too many place. So I prefer my initial patch.

ruiu accepted this revision.Dec 7 2016, 3:50 PM
ruiu edited edge metadata.

LGTM

This revision is now accepted and ready to land.Dec 7 2016, 3:50 PM
This revision was automatically updated to reflect the committed changes.