[PATCH] D116881: [ELF] Add RelocationScanner. NFC

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/ELF/Relocations.cpp:408
+namespace {
+class Resolver {
+public:
----------------
peter.smith wrote:
> I normally think of resolving a relocation when the final value is written. Given that the only public member function is called `scanRelocs`. Perhaps call the class `Scanner` or `RelocationScanner` the latter is more verbose but we could use `scan` instead of `scanRelocs`.
> 
> Regardless of what the name is, it will be good to have a comment to describe this large class. 
Changed to `RelocationScanner`. I do not remember how I picked Resolver...


================
Comment at: lld/ELF/Relocations.cpp:1285
   const RelTy &rel = *i;
-  uint32_t symIndex = rel.getSymbol(config->isMips64EL);
+  uint32_t symIndex = rel.getSymbol(0);
   Symbol &sym = sec.getFile<ELFT>()->getSymbol(symIndex);
----------------
ikudrin wrote:
> Why `config->isMips64EL` is changed to `0`?
Fixed.

It was unintentional - I was testing how mips checks affect the performance.

@atanasyan There is missing coverage for mips64el.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116881/new/

https://reviews.llvm.org/D116881