[PATCH] D109516: [JITLink] Factor out forEachRelocation() function from addRelocations() in ELF Aarch64 backend (NFC)

[Stefan Gränitz via Phabricator via llvm-commits]

sgraenitz added a comment.

In D109516#2995913 <https://reviews.llvm.org/D109516#2995913>, @lhames wrote:

> Is there a reason that forEachRelocation works one-section-at-a-time? I thought that forEachRelocation could embed the loop over the sections within it to simplify addRelocations further.

I started like this and then realized that the x86_64 backend does an additional check on the type of the relocation section: https://github.com/llvm/llvm-project/blob/15e9575fb5988a66aa6e57a55818b54b575dd795/llvm/lib/ExecutionEngine/JITLink/ELF_x86_64.cpp#L217 Not sure if this specific use-case is really relevant, but I started thinking it might be worth having this option. (Especially since, logically, this loop doesn't add any value to the forEachRelocation function.)

Thinking again now: Assuming we skip most sections immediately, this probably adds unreasonable overhead. Because now there's 2 function calls before checking the type against `ELF::SHT_REL(A)`. And there's not much hope for inlining even with the templated parameter, right? Would be really bad with `-ffunction-sections`.

So, yes for performance reasons I'd change that and put the loop back inside. @lhames What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109516