[PATCH] D115993: [ELF] Optimize RelocationSection<ELFT>::writeTo

Igor Kudrin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 20 07:30:01 PST 2021


ikudrin added a comment.





================
Comment at: lld/ELF/SyntheticSections.cpp:1677-1679
+    rel.addend = rel.computeAddend();
+    rel.r_offset = rel.getOffset();
+    rel.r_sym = rel.getSymIndex(symTab);
----------------
Preferably, that should be a method in the `DynamicReloc` class.


================
Comment at: lld/ELF/SyntheticSections.h:480
+  Symbol *sym;
+  const OutputSection *outputSec = nullptr;
+  const InputSectionBase *inputSec;
----------------
peter.smith wrote:
> Just thinking about making the enum size uint8_t Not entirely sure it will make a lot of difference in this case, but you may want to reorder some of the fields so that they minimise padding between items. For example addend before r_sym. Kind may also benefit from being last. 
`RelType` is `uint32_t`, so there is no padding. Moving `Kind` at the end (as well as making it `uint8_t`) changes nothing as the whole class should be 64-bit aligned and thus is 64-bytes long regardless the changes.


================
Comment at: lld/ELF/SyntheticSections.h:488
+  // RelocationSection<ELFT>::writeTo.
+  int64_t addend;
 
----------------
I do not personally like optimizations when the meaning of the field is changed in different phases. That makes the code fragile and hard to support.

At least, when `addend` is updated, `kind` should be adjusted, or else the object's state becomes inconsistent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115993



More information about the llvm-commits mailing list