[PATCH] D133003: [ELF] Parallelize relocation scanning

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

No objections from me. Some small suggestions for comments and a way that might catch someone using the relocs array before mergeRels has been called.



================
Comment at: lld/ELF/SyntheticSections.h:507
+                        RelType addendRelType, RelExpr expr) {
+    // This function should only be called for non-preemptible symbols or
+    // RelExpr values that refer to an address inside the output file (e.g. the
----------------
Would it be better to move the text into the /// comment as it is a precondition for calling the function?


================
Comment at: lld/ELF/SyntheticSections.h:546
   int32_t dynamicTag, sizeDynamicTag;
   SmallVector<DynamicReloc, 0> relocs;
+  // Used when parallel relocation scanning adds relocations. The elements
----------------
Now that mergeRels has to be called before this is useable, is it worth making this private with an interface that asserts mergeRels has been called?


================
Comment at: lld/ELF/SyntheticSections.h:547
   SmallVector<DynamicReloc, 0> relocs;
+  // Used when parallel relocation scanning adds relocations. The elements
+  // will be moved into relocs.
----------------
Suggest "// will be moved into relocs by mergeRels()."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133003