[PATCH] D108986: [JITLink] Add initial Aarch64 support

[Lang Hames via Phabricator via llvm-commits]

lhames accepted this revision.
lhames added a comment.
This revision is now accepted and ready to land.

This looks great to me. Thank you very much Stefan!

> This patch follows the idea to keep implementations for ARM (32-bit) and Aaarch64 (64-bit) separate, because:
> it might be easier to share code with the MachO "arm64" JITLink backend
> LLVM has individual targets for ARM and Aaarch64 as well

Yes -- This is the right approach. We'll want to unify the AArch64 edge set the same way that we did for x86-64.



================
Comment at: llvm/lib/ExecutionEngine/JITLink/ELF_aarch64.cpp:88-98
+  Error addRelocations() override {
+    using Base = ELFLinkGraphBuilder<ELFT>;
+    LLVM_DEBUG(dbgs() << "Adding relocations\n");
+
+    // Iterate sections and only process the interesting ones.
+    for (auto &SecRef : Base::Sections) {
+      if (SecRef.sh_type != ELF::SHT_RELA && SecRef.sh_type != ELF::SHT_REL)
----------------
There is a lot of redundancy in these `addRelocations` functions. Do you think we could add a generic function (or functions) to the ELFLinkGraphBuilder so that clients can just write:

```
for_all_relas([](const Rela &R) {
  // Handle rela
});
```

?

I don't think this is something that we should do for this review, but it should be done either before or after it lands so that future backends don't keep copying this code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108986