[MC] Requiring MCAsmInfo for MCExpr evaluation?

Jim Grosbach grosbach at apple.com
Fri Apr 24 11:01:28 PDT 2015


SGTM.

If we change this universally (x86 and ARM, too), do you have any thoughts on whether this would actually break anything? And more importantly, anything that we couldn’t straightforwardly fix? I’d really prefer to just make this an across the board fix if possible.

-Jim

> On Apr 24, 2015, at 10:42 AM, Ahmed Bougacha <ahmed.bougacha at gmail.com> wrote:
> 
> Jim, Tim, Kevin: I want to change the MCExpr Shr constant evaluation
> to be either logical or arithmetic, depending on the target.
> 
> It's always arithmetic currently, and 1) that doesn't match current
> gas; 2) it leads to unexpected behavior like:
> 
>    $ echo 'movz x1, ((0xfffffffffffffffc & 0xffff000000000000) >>
> 48), LSL #48' | llvm-mc -arch aarch64
> 
>    <stdin>:1:10: error: immediate must be an integer in range [0, 65535].
>    movz x1, ((0xfffffffffffffffc & 0xffff000000000000) >> 48), LSL #48
> 
>           (from https://llvm.org/bugs/show_bug.cgi?id=23227)
> 
> I believe it makes sense to update all targets, but I think that ship
> has sailed for X86/ARM Darwin, where there's a point to be made for
> backwards compatibility with gas 1.38 (`as -Q').
> 
> For AArch64 Darwin however, would it make sense to use LShr?
> 
> Thanks,
> -Ahmed
> 
> On Fri, Apr 24, 2015 at 8:07 AM, Rafael Espíndola
> <rafael.espindola at gmail.com> wrote:
>> +  bool useLogicalShr()
>> 
>> shouldUseLogicalShr() is probably a bit better.
>> 
>> 
>> This changes the behaviour of AArch64 MachO, right? Can we leave that
>> as is for now and change it in another patch?
>> 
>> 
>> On 22 April 2015 at 20:49, Ahmed Bougacha <ahmed.bougacha at gmail.com> wrote:
>>> On Wed, Apr 22, 2015 at 2:43 PM, Rafael Espíndola
>>> <rafael.espindola at gmail.com> wrote:
>>>> Looks like we have to store more information in the expressions
>>>> themselves. Would changing
>>>> 
>>>> Lhr,  ///< Shift right (arithmetic or logical, depending on target)
>>>> 
>>>> to
>>>> 
>>>> Lshr,  ///< Logical shift right
>>>> Ashr,  ///< Arithmetic shift right
>>>> 
>>>> solve the problem?
>>> 
>>> Yup, how about the attached patch?
>>> 
>>> I tried binutils v2.24 aarch64-linux-gnu-as, and it has the same
>>> behavior.  On Darwin, the only AArch64 assembler is LLVM, and the
>>> previous behavior was wrong, so the change is fine.
>>> 
>>> We'll see about other targets.  I looked at the assemblers I had lying
>>> around (ARM, AArch64, X86; darwin, linux) and AFAIK, only ancient GAS
>>> (I tried the last Apple-bundled one, v1.38) does the AShr.
>>> 
>>> Notably, binutils v2.24 for arm-linux-gnueabihf and x86_64-linux-gnu
>>> both do the LShr.  So it's probably fine to do the switch there too,
>>> at the very least?
>>> 
>>> Are there even targets where AShr is the intended behavior, and not
>>> some 20yo GAS bug?
>>> 
>>> -Ahmed
>>> 
>>>> Cheers,
>>>> Rafael





More information about the llvm-commits mailing list