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

[Jim Grosbach]

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.

-Jim


On Mar 21, 2012, at 4:06 PM, Jim Grosbach <grosbach at apple.com> wrote:

> This should be handled in the ELF relocation code. The encoder is for more than just ELF and this will break Darwin horribly.
> 
> -Jim
> 
> 
> On Mar 19, 2012, at 6:47 AM, Mans Rullgard <mans at mansr.com> wrote:
> 
>> Using fixup_arm_bl results in an R_ARM_CALL relocation which is not
>> allowed for conditional BL instructions.
>> ---
>> lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp |    6 +++++-
>> 1 files changed, 5 insertions(+), 1 deletions(-)
>> 
>> diff --git a/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp b/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
>> index 4445dcd..cf18508 100644
>> --- a/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
>> +++ b/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
>> @@ -597,8 +597,12 @@ uint32_t ARMMCCodeEmitter::
>> getARMBLTargetOpValue(const MCInst &MI, unsigned OpIdx,
>>                          SmallVectorImpl<MCFixup> &Fixups) const {
>>  const MCOperand MO = MI.getOperand(OpIdx);
>> -  if (MO.isExpr())
>> +  if (MO.isExpr()) {
>> +    if (HasConditionalBranch(MI))
>> +      return ::getBranchTargetOpValue(MI, OpIdx,
>> +                                      ARM::fixup_arm_condbranch, Fixups);
>>    return ::getBranchTargetOpValue(MI, OpIdx, ARM::fixup_arm_bl, Fixups);
>> +  }
>> 
>>  return MO.getImm() >> 2;
>> }
>> -- 
>> 1.7.8.5
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits