[PATCH] D90614: [lld-macho][nfc] Refactor to accommodate paired relocs

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added inline comments.


================
Comment at: lld/MachO/InputFiles.cpp:178
+
+  for (size_t i = 0; i < relInfos.size(); i++) {
+    const relocation_info pairedInfo = relInfos[i];
----------------
gkm wrote:
> int3 wrote:
> > other reviewers have mentioned that LLVM style is to evaluate loop constants in the assignment part of the loop header (i.e. `size_t i= 0, e = < relInfos.size(); i < e; ...`). I don't really care much either way, just FYI
> I did a quick audit for this in lld. The recommended LLVM style is used approx 1/3 of the time.
I was curious about this so I looked at the generated assembly (my host compiler was Clang 9). At least in this case, it's smart enough to realize that the loop constant is, well, constant (and that it's just `sec.nreloc`) and uses that constant value instead of recomputing things in each iteration. (The value gets spilled to the stack so it's still a memory reload, but the extra variable wouldn't have saved you either.) I can imagine that analysis being a lot harder to do for the compiler in the general case though, hence the LLVM style recommendation (but yeah, it doesn't seem too enforced in LLD).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90614