[PATCH] D157540: [JITLink][AArch32] Fixes for initial AArc32 backend

Mon Sep 4 05:13:16 PDT 2023

sgraenitz accepted this revision.
sgraenitz added a subscriber: peter.smith.
sgraenitz added a comment.
This revision is now accepted and ready to land.

Ok, I finally double-checked it and you are right. Bit 12 must be set and not clear! Can we rename the field to `LoBitUnconditional` and change the condition into `if (!(R.Lo & FixupInfo<Thumb_Jump24>::LoBitUnconditional))`?

Additionally, I am afraid there is more to fix here. The docs <https://github.com/ARM-software/abi-aa/blob/main/aaelf32/aaelf32.rst> say:

> There is one relocation (R_ARM_CALL) for unconditional function call instructions (BLX and BL with the condition field set to 0xe), and one for jump instructions (R_ARM_JUMP24). [...] Conditional function call instructions (BL<cond>) must be relocated using R_ARM_JUMP24.

I assume the same holds for the respective Thumb instructions? (Do you know @peter.smith?) So, the `Thumb_Jump24` relocation is the one that supports conditional branches and `Thumb_Call` is the one that should have this check :) I leave it to you whether or not to implement this now. We can also do it later and for now only rename the field and fix the condition. Everything else is fine. Thanks for working on this!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157540