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

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

In D157540#4636960 <https://reviews.llvm.org/D157540#4636960>, @sgraenitz wrote:

> 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!

Thumb (2) has some similarities and some differences. Like Arm there are two relocations R_ARM_THM_JUMP24 for B and a R_ARM_THM_CALL (BL and BLX). However in Thumb state no instructions are conditional they are made conditional by an IT block. So you can have something like:

          .text
          .thumb
          it eq
          bleq foo // R_ARM_THM_CALL
  
          .arm
          .type foo, %function
          .balign 4
  foo:
          bx lr

Which at link time will end up as:

  00008000 <$t.0>:
      8000: bf08          it      eq
      8002: f000 e802     blxeq   0x8008 <foo>            @ imm = #0x4
      8006: 0000          movs    r0, r0
  
  00008008 <foo>:
      8008: e12fff1e      bx      lr

So in Thumb state the conditional bl can use the R_ARM_THM_CALL. Which it can't do in Arm state. I would recommend not generating a conditional BL in Arm state if possible as it means that if interworking is needed a full veneer/thunk/stub (whatever it is called in the tool) is needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157540