[llvm-commits] [PATCH] ARM: conditional BL should use fixup_arm_condbranch

[Måns Rullgård]

Jim Grosbach <grosbach at apple.com> writes:

> Hi Mans,
>
> Sorry for the previous terseness. What I get for replying when I'm in
> a hurry. I phrased things poorly and probably implied a bit of a
> different issue than I intended.
>
> The way this works is that the fixups direct the behavior of the
> encoder and when different encoding is required, be it bit twiddling
> or a different relocation, a different fixup is used. Typically this
> sort of thing is platform agnostic so we can use the same fixups for
> each. In this case, however, we're not so lucky. What we want to do is
> check the platform in the encoder and select the appropriate
> fixup. That will then allow the ELFObjectWriter and the
> MachObjectWriter to do the correct platform specific thing.
>
> FWIW, the Darwin problem with using a conditional branch fixup is that
> Darwin relies on a relocation to get interworking correct, and
> conditional fixups can (and do) get resolved statically by the
> assembler. The result is an incorrect mode switch in tail-recursive
> functions.

In my case, the branch could have been resolved statically, but the ELF
object writer emitted an R_ARM_CALL relocation anyway, which confused
the GNU linker.  ELF relocations of calls and jumps are perhaps best
described by the spec:

  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). The principal difference between
  the two relocation values is the handling of ARM/Thumb interworking:
  on ARM architecture 5 and above, an instruction relocated by
  R_ARM_CALL that calls a function that is entered in Thumb state may be
  relocated by changing the instruction to BLX; an instruction relocated
  by R_ARM_JUMP24 must use a veneer to effect the transition to Thumb
  state. Conditional function call instructions (BL<cond>) must be
  relocated using R_ARM_JUMP24.

This means we need to distinguish between two classes of branch
relocations:

- Unconditional immediate calls, which can be fully handled by exchanging
  the BL instruction for a BLX if a mode switch is
  required. (R_ARM_CALL)

- Non-call branches and conditional immediate calls, which require a
  veneer for mode switching. (R_ARM_JUMP24)

By using the same fixup kind for both, the distinction is lost in the
object writers.  Perhaps adding another fixup kind for conditional calls
(fixup_arm_condbl?) would be a solution.  The Mach-o writer could handle
this identically to fixup_arm_bl, while the ELF writer would select the
appropriate relocation type.  To me this seems cleaner than littering
the otherwise generic code emitter with platform-specifics.

-- 
Måns Rullgård
mans at mansr.com