[PATCH] D144083: [JITLink] Initial AArch32 backend

[Lang Hames via Phabricator via llvm-commits]

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

Relocation names should be generic (at least for `getEdgeKindName`), but otherwise LGTM!

Thank you very much for this Stefan: it's a huge contribution, with very neat solutions to hard problems, and a big step forward for LLVM JIT support!



================
Comment at: llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp:78-89
+const char *getEdgeKindName(Edge::Kind Kind) {
+  auto ELFType = getELFRelocationType(Kind);
+  if (!ELFType)
+    return getGenericEdgeKindName(Kind);
+  StringRef Name = object::getELFRelocationTypeName(ELF::EM_ARM, *ELFType);
+
+  // FIXME: In order to match the LinkGraph::GetEdgeKindNameFunction interface,
----------------
sgraenitz wrote:
> lhames wrote:
> > This should return the name of the JITLink edge enum, rather than the ELF relocation. That's not a blocker for landing though.
> Do we need the name of the internal edge-kind? I moved the function here from `aarch32.h` to allow format-specific output. (I still have to see how it works out with intermediate edge kinds.)
The purpose of `getEdgeKindName` is definitely to name the Edge::Kind enum values (to make it easy to look them up in the JITLink docs and applyRelocation), but I think it makes sense to add support for a reverse mapping to the original relocation names too. We should introduce a new function for this, but that can be done post-commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144083