[PATCH] D144083: [JITLink] Initial AArch32 backend

[Peter Smith via Phabricator via llvm-commits]

peter.smith added inline comments.


================
Comment at: llvm/lib/ExecutionEngine/JITLink/aarch32.cpp:244
+      return makeUnexpectedOpcodeError(G, R, Kind);
+    if ((R.Lo & FixupInfo<Thumb_Call>::LoBitNoBlx) == 0 &&
+        (R.Lo & FixupInfo<Thumb_Call>::LoBitH) != 0)
----------------
sgraenitz wrote:
> peter.smith wrote:
> > Looking at the stub generation code, if there is a Thumb_Call to an external edge, then a stub with LoBitH will be set will be created. This won't be a problem if the code-generator never generates BLX to an external symbol, but would be if it does.
> Yes, I didn't catch a case like this yet. But I am also not sure I fully understand: If codegen wrote a BLX, then it would be T2 right? The docs say LoBitH must be empty here:
> ```
> if CurrentInstrSet() == InstrSet_ThumbEE || H == '1' then UNDEFINED;
> ```
> 
> What is the stub generation code you mean? The one here in JITLink?
Yes, the code in visitEdge, quoted here. If there is a BLX instruction with a Thumb_Call relocation where the target isn't defined, it looks like a stub will be created.

```
bool visitEdge(LinkGraph &G, Block *B, Edge &E) {
    if (E.getTarget().isDefined())
      return false;

    switch (E.getKind()) {
    case Thumb_Call:
    case Thumb_Jump24: {
      DEBUG_WITH_TYPE("jitlink", {
        dbgs() << "  Fixing " << G.getEdgeKindName(E.getKind()) << " edge at "
               << B->getFixupAddress(E) << " (" << B->getAddress() << " + "
               << formatv("{0:x}", E.getOffset()) << ")\n";
      });
      E.setTarget(this->getEntryForTarget(G, E.getTarget()));
      return true;
    }
    }
    return false;
  }
```
I would not expect a code-generator to generate a BLX in this situation as it has no idea whether the target is Arm or Thumb so I would expect a BL. It is possible to write in assembly language but I don't know how well that needs to be supported in a JIT.

In static linkers an R_ARM_CALL (Arm state) or R_ARM_THM_CALL (Thumb state) relocation can be associated with a BL or BLX. As the static linker always knows the (Arm/Thumb) state of the destination it can write a BL instruction if both source and target are the same state or a BLX if they are different. This property permits code-generators to only use BL with R_ARM_CALL/R_ARM_THM_CALL, with BLX reserved for assembler. It also permits Thumb code to reuse Arm stubs.

I don't know how often this would happen in a JIT though.


================
Comment at: llvm/lib/ExecutionEngine/JITLink/aarch32.cpp:261
+                                      StringRef(G.getEdgeKindName(Kind)));
+    return decodeImmBT4BlT1BlxT2_J1J2(R.Hi, R.Lo);
+
----------------
sgraenitz wrote:
> peter.smith wrote:
> > Is there a reason why this isn't
> > ```
> > LLVM_LIKELY(ArmCfg.J1J2BranchEncoding)
> >                ? decodeImmBT4BlT1BlxT2_J1J2(R.Hi, R.Lo)
> >                : decodeImmBT4BlT1BlxT2(R.Hi, R.Lo);
> > ```
> > The J1J2 would apply here in the same way as Thumb_Call.
> My understanding was that CPUs without J1J2 didn't have R_ARM_THM_JUMP24 yet, so it wouldn't every happen? (Maybe that would be worth an assertion.)
R_ARM_THM_JUMP24 is used for the wide unconditional branch regardless of whether J1J2 encoding is available. It was introduced when the BLX instruction was made available. BL <immediate> can be converted into BLX <immediate> but B.w <immediate> cannot be converted in BX <immediate> as BX <immediate> doesn't exist (only BX <register>).

In summary R_ARM_THM_CALL is for BL and BLX instructions, R_ARM_THM_JUMP24 is for B.w instructions. The J1J2 encoding applies to both BL, BLX and B.w.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144083