[PATCH] D113228: [RFC][ELF] Refactor relocation processing

Stefan Pintilie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 10 13:26:07 PST 2021


stefanp added a comment.

I feel like there is a lot going on in this patch. It may be easier if it was just done little by little.
For example: `toExecRelax` is now a static function. This can be separated out into a small NFC patch where the only thing that is done is the refactoring of that one static function. 
Also, `handleTlsRelocation` returns a slightly different value (what should be skipped vs what was processed). This can also be done as a separate mini patch.

Anyway, those are my 2 cents.



================
Comment at: lld/ELF/Relocations.cpp:1269-1271
   }
 
   return 0;
----------------
Do we need this `}` and the `return 0;` ?
The if has been changed to an assert so now we just return `return target->getTlsGdRelaxSkip(type) - 1;`. 

You can also remove the starting `}`. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113228



More information about the llvm-commits mailing list