The current support for V1 ABI in LLD is incomplete. This patch removes V1 ABI support and changes the default behavior to V2 ABI, issuing an error when using the V1 ABI. It also updates the testcases to V2 and removes any V1 specific tests.
Details
- Reviewers
sfertile ruiu rdhindsa • espindola - Commits
- rGedc7a8c1e5c0: [PPC64] Remove support for ELF V1 ABI in LLD - buildbot fix
rGf61b0733a854: [PPC64] Remove support for ELF V1 ABI in LLD
rL331534: [PPC64] Remove support for ELF V1 ABI in LLD - buildbot fix
rLLD331534: [PPC64] Remove support for ELF V1 ABI in LLD - buildbot fix
rL331529: [PPC64] Remove support for ELF V1 ABI in LLD
rLLD331529: [PPC64] Remove support for ELF V1 ABI in LLD
Diff Detail
Event Timeline
lld/ELF/Arch/PPC64.cpp | ||
---|---|---|
101–105 | No, there is no restriction that V2 is LE only. |
lld/ELF/Arch/PPC64.cpp | ||
---|---|---|
93–94 | We need to keep both LE and BE. |
lld/ELF/Arch/PPC64.cpp | ||
---|---|---|
93–94 | Oh okay. I thought PPCv2 ABI is LE only. I stand corrected. I'm curious if compiler tool chain actually supports BE PPCv2 ABI. | |
101–107 | I don't think you need this assert. | |
103 | Instead of Config->EKind == ELF64LEKind, please use Config->IsLE. | |
106 | Perhaps EFlags > 2 is a bit better. | |
lld/ELF/InputSection.cpp | ||
562–563 | You return here only for PPC64. Is this correct? We generally should avoid this because it is sometimes hard to find that only one branch of cascaded if returns immediately. If this is really correct, I'd add if (Sym.isUndefWeak() && Config->EMachine == EM_PPC64) return 0; but I'm not sure if that's correct. |
lld/ELF/InputSection.cpp | ||
---|---|---|
562–563 | This was previously handled with R_PPC_OPD which returned 0 for an undefined weak symbol by checking if the symbol address is zero. Now I have removed R_PPC_OPD and use this relocation, keeping the same return 0 behavior as before. |
lld/ELF/InputSection.cpp | ||
---|---|---|
562–563 | Doesn't it conflict with the existing behavior for R_PC? You are changing the default behavior of R_PC. If returning 0 makes sense only for R_PPC_OPD, I think you need to keep it. |
lld/ELF/InputSection.cpp | ||
---|---|---|
562–563 | Keeping it, but changing name to not include "OPD" which is part of V1 abi |
LGTM with these changes.
lld/ELF/Arch/PPC64.cpp | ||
---|---|---|
105 | Perhaps it is better to return 0 for an invalid version number to prevent cascading error messages. Currently, if you return, say 3, you'll see something like "ABI version 3 is not compatible with ..." error after this error message. | |
lld/ELF/Relocations.h | ||
59–62 | Sort |
Please update this as well for LE only.