[PATCH] D157533: [JITLink][AArch32] Implement ELF::R_ARM_CALL relocation

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

sgraenitz added a comment.

Looks great, thanks! I think at some point we should add a `.thumb` section with `.thumb_func` functions to our tests, so we get coverage for the code that handles ARM-Thumb transitions. However, I guess we need ARM stubs for it first?



================
Comment at: llvm/lib/ExecutionEngine/JITLink/aarch32.cpp:294
   uint64_t TargetAddress = TargetSymbol.getAddress().getValue();
-  assert(!TargetSymbol.hasTargetFlags(ThumbSymbol));
 
----------------
Oh right, this has to go away!


================
Comment at: llvm/lib/ExecutionEngine/JITLink/aarch32.cpp:424
+        R.Wd = R.Wd & ~FixupInfo<Arm_Call>::BitH;
+        // Value = alignTo(Value, 4);
+        // Set Thumb bit
----------------
I think we don't need to worry about alignment here, because Thumb requirements are lower than ARM. Ideally we'd have test coverage though :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157533