This patch optimizes the global variable access sequence when R_PPC64_TOC16_HA evaluates to 0.
The addis that the R_PPC64_TOC16_HA relocation operates on is overwritten with a nop and the related instruction that uses either a *_LO or *_LO_DS form is update to use the toc pointer a base register.
Details
Diff Detail
- Repository
- rLLD LLVM Linker
Event Timeline
Overall I think the patch looks good, but we should expand the ppc64-toc-addis-nop.s test to cover all the different sequences this will affect (like loading bytes/half-word/word signed and unsigned etc).
lld/ELF/Arch/PPC64.cpp | ||
---|---|---|
473 ↗ | (On Diff #155181) | Should the mask for ds-form instructions be 0xFFE00003 instead? I think this will turn something like lwa 3,b@toc@l(4) into ld 3, b@toc@l(2) |
lld/ELF/Arch/PPC64.cpp | ||
---|---|---|
390–391 ↗ | (On Diff #155181) | return Type == R_PPC64_TOC16_HA || Type == R_PPC64_TOC16_LO_DS || Type == R_PPC64_TOC16_LO; is better. |
399 ↗ | (On Diff #155181) | I don't think you have to call this function ahead of time. Instead, I'd replace IsTocRelType with isTocRelType(Type). I believe compiler can optimize it so that it is as fast as this code. |
- Error when optimizing an updated form load or store.
- Add an option that can disable the toc optimization.
- Expand the testing.
ELF/Arch/PPC64.cpp | ||
---|---|---|
132 | This variable is used only once. You can eliminate it. | |
558 | Just return the result of the boolean expression instead of if (e) return true; return false; | |
629 | Can't -> can't | |
630 | We have llvm::utohexstr, so I believe you can omit Twine::. | |
ELF/Driver.cpp | ||
287–288 | It is indeed true that --no-toc-optimize is PPC64 only, but in other test patterns we don't care about that kind of negative flags. I'd check if Config->TocOptimize is true. | |
ELF/Options.td | ||
319–320 | Indentation. Shouldn't TOC be all-caps? |
Moved checking of OPT_toc_optimise until after the machine has been decided and made it default to on only for PowerPC64.
ELF/Driver.cpp | ||
---|---|---|
287–288 | Forgot to update that in the diff, its fixed now. |
This variable is used only once. You can eliminate it.