[llvm-commits] [llvm] r123294 - in /llvm/trunk: lib/MC/ELFObjectWriter.cpp lib/Target/ARM/ARMAsmBackend.cpp lib/Target/ARM/ARMAsmPrinter.cpp lib/Target/ARM/ARMFixupKinds.h lib/Target/ARM/ARMMCCodeEmitter.cpp test/MC/ARM/elf-movt.s

[Evan Cheng]

On Jan 12, 2011, at 4:23 PM, Jason Kim wrote:

> On Wed, Jan 12, 2011 at 3:56 PM, Evan Cheng <evan.cheng at apple.com> wrote:
>> 
>> On Jan 12, 2011, at 2:34 PM, Jason Kim wrote:
>> 
>>>> 
>>> 
>>> The long story is quite convoluted, but the simple answer is that when
>>> the "immediate value" being loaded to a register via movw/movt is
>>> itself an expression that evaluates to a PCrelative value,
>>> GNU as creates two different relocation values for the MOVW/MOVT. This
>>> patch is to address that case.
>>> It just so happens that some 99.999% of such addresses, the
>>> expressions are of the form:
>>> 
>>> movw r0, :lower16:Foo-(Bar+8)
>>> movt  r0, :upper16:Foo-(Bar+8)
>> 
>> Is this what gcc generates? That can't be right.
>> 
>> movw r0, :lower16:Foo-(Bar+8)
>> shouldn't be mean
>> movw r0, :lower16:(Foo-(Bar+8))
> 
> Yes canonically speaking the latter is the least confusing of the two

ok

> - GNU as accepts both AFAIK.

I hope it generates different code for the two cases. Otherwise, it's a bug in my opinion.

Evan

> -Jason
> 
>> 
>> Evan
>> 
>>> 
>>> For such expressions, GNU as creates two different relocations with
>>> the reloc tags
>>> R_ARM_MOVW_PREL_NC and  R_ARM_MOVT_PREL,
>>> In normal cases where the expression is not a pcrel value, GNU as
>>> creates R_ARM_MOVT_ABS and R_ARM_MOVW_ABS_NC
>>> 
>>> The reason why that hack routine works is because in nearly all cases,
>>> the expression winds up being a highly constrained Binary expression -
>>> LLVM also constrains this because MCValue has room for exactly two
>>> symbols (but in reality, the pcrel expression can be hairy beasts like
>>> Foo+Bar-Baz + 8 etc... but llvm doesn't seem to support this directly)
>>> 
>>>> 
>>>>> +static bool EvaluateAsPCRel(const MCExpr *Expr) {
>>>>> +  switch (Expr->getKind()) {
>>>>> +  case MCExpr::SymbolRef: return false;
>>>>> +  case MCExpr::Binary: return true;
>>>>> +  default: assert(0 && "Unexpected expression type");
>>>>> +  }
>>>>> +}
>>>>> +
>>>>> uint32_t ARMMCCodeEmitter::
>>>>> getMovtImmOpValue(const MCInst &MI, unsigned OpIdx,
>>>>>                   SmallVectorImpl<MCFixup> &Fixups) const {
>>>>> @@ -635,18 +661,27 @@
>>>>>   if (MO.isImm()) {
>>>>>     return static_cast<unsigned>(MO.getImm());
>>>>>   } else if (const MCSymbolRefExpr *Expr =
>>>>> -             dyn_cast<MCSymbolRefExpr>(MO.getExpr())) {
>>>>> +             FindLHSymExpr(MO.getExpr())) {
>>>>> +    // FIXME: :lower16: and :upper16: should be applicable to
>>>>> +    // to whole expression, not just symbolrefs
>>>>> +    // Until that change takes place, this hack is required to
>>>>> +    // generate working code.
>>>>> +    const MCExpr *OrigExpr = MO.getExpr();
>>>>>     MCFixupKind Kind;
>>>>>     switch (Expr->getKind()) {
>>>>>     default: assert(0 && "Unsupported ARMFixup");
>>>>>     case MCSymbolRefExpr::VK_ARM_HI16:
>>>>>       Kind = MCFixupKind(ARM::fixup_arm_movt_hi16);
>>>>> +      if (EvaluateAsPCRel(OrigExpr))
>>>>> +        Kind = MCFixupKind(ARM::fixup_arm_movt_hi16_pcrel);
>>>>>       break;
>>>>>     case MCSymbolRefExpr::VK_ARM_LO16:
>>>>>       Kind = MCFixupKind(ARM::fixup_arm_movw_lo16);
>>>>> +      if (EvaluateAsPCRel(OrigExpr))
>>>>> +        Kind = MCFixupKind(ARM::fixup_arm_movw_lo16_pcrel);
>>>>>       break;
>>>>>     }
>>>>> -    Fixups.push_back(MCFixup::Create(0, Expr, Kind));
>>>>> +    Fixups.push_back(MCFixup::Create(0, OrigExpr, Kind));
>>>>>     return 0;
>>>>>   };
>>>>>   llvm_unreachable("Unsupported MCExpr type in MCOperand!");
>>>>> 
>>>>> Modified: llvm/trunk/test/MC/ARM/elf-movt.s
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/ARM/elf-movt.s?rev=123294&r1=123293&r2=123294&view=diff
>>>>> ==============================================================================
>>>>> --- llvm/trunk/test/MC/ARM/elf-movt.s (original)
>>>>> +++ llvm/trunk/test/MC/ARM/elf-movt.s Tue Jan 11 18:19:25 2011
>>>>> @@ -1,4 +1,6 @@
>>>>> @ RUN: llvm-mc %s -triple=armv7-linux-gnueabi | FileCheck -check-prefix=ASM %s
>>>>> +@ RUN: llvm-mc %s -triple=armv7-linux-gnueabi -filetype=obj -o - | \
>>>>> +@ RUN:    elf-dump --dump-section-data | FileCheck -check-prefix=OBJ %s
>>>>>       .syntax unified
>>>>>       .text
>>>>>       .globl  barf
>>>>> @@ -12,3 +14,26 @@
>>>>> @ ASM:          movw    r0, :lower16:GOT-(.LPC0_2+8)
>>>>> @ ASM-NEXT:     movt    r0, :upper16:GOT-(.LPC0_2+16)
>>>>> 
>>>>> +@@ make sure that the text section fixups are sane too
>>>>> +@ OBJ:                 '.text'
>>>>> +@ OBJ-NEXT:            'sh_type', 0x00000001
>>>>> +@ OBJ-NEXT:            'sh_flags', 0x00000006
>>>>> +@ OBJ-NEXT:            'sh_addr', 0x00000000
>>>>> +@ OBJ-NEXT:            'sh_offset', 0x00000034
>>>>> +@ OBJ-NEXT:            'sh_size', 0x00000008
>>>>> +@ OBJ-NEXT:            'sh_link', 0x00000000
>>>>> +@ OBJ-NEXT:            'sh_info', 0x00000000
>>>>> +@ OBJ-NEXT:            'sh_addralign', 0x00000004
>>>>> +@ OBJ-NEXT:            'sh_entsize', 0x00000000
>>>>> +@ OBJ-NEXT:            '_section_data', 'f00f0fe3 ec0f4fe3'
>>>>> +
>>>>> +@ OBJ:              Relocation 0x00000000
>>>>> +@ OBJ-NEXT:         'r_offset', 0x00000000
>>>>> +@ OBJ-NEXT:         'r_sym'
>>>>> +@ OBJ-NEXT:         'r_type', 0x0000002d
>>>>> +
>>>>> +@ OBJ:              Relocation 0x00000001
>>>>> +@ OBJ-NEXT:         'r_offset', 0x00000004
>>>>> +@ OBJ-NEXT:         'r_sym'
>>>>> +@ OBJ-NEXT:         'r_type', 0x0000002e
>>>>> +
>>>>> 
>>>>> 
>>>>> _______________________________________________
>>>>> 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
>> 
>>