[PATCH] D73254: [ELF] Rename relocateOne() to relocate() and pass `Relocation` to it
Nick Desaulniers via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 23 09:34:05 PST 2020
nickdesaulniers added a comment.
Thanks for the patch!
================
Comment at: lld/ELF/Target.h:91
+ relocate(loc, rel, val);
+ }
----------------
It looks like there's just a few more callers of `relocateOne` in:
* ELF/AArch64ErrataFix.cpp
* ELF/ARMErrataFix.cpp
* ELF/InputSection.cpp
I think if you updated those too, you could drop this change to `relocateOne` and then this patch would be simply renaming `relocateOne` to `relocate` and taking a full `Relocation` object rather than a `RelType`.
You might even give `struct Relocation` an non-`explicit` constructor that way you could implicitly construct a `Relocation` via a `RelType`. Then calls to `relocateOne` with a literal `RelType` would only have to be renamed, and you wouldn't have to add code to contruct `Relocations` at callsites.
(It looks like `struct Relocation` in llvm/tools/llvm-objcopy/COFF/Object.h already does something to this effect, for prior art). Oh man, so many different `Relocation` structs across the codebase...
================
Comment at: lld/ELF/Target.h:246
error(getErrorLocation(loc) + "improper alignment for relocation " +
- lld::toString(type) + ": 0x" + llvm::utohexstr(v) +
+ lld::toString(rel.type) + ": 0x" + llvm::utohexstr(v) +
" is not aligned to " + Twine(n) + " bytes");
----------------
Are the changes to this file necessary? Is the full `Relocation` object used?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73254/new/
https://reviews.llvm.org/D73254
More information about the llvm-commits
mailing list