RE: [PATCH][ARM]Tighten the conditions for arm_movw, arm_movt

["Kyrill Tkachov" <kyrylo dot tkachov at arm dot com>]

Hi Renlin,

On 19/08/15 15:37, Renlin Li wrote:
> Hi Kyrylo,
>
> On 19/08/15 13:46, Kyrylo Tkachov wrote:
>> Hi Renlin,
>>
>> Please send patches to gcc-patches for review.
>> Redirecting there now...
> Thank you! I should really double check after Thunderbird auto complete 
> the address for me.
>
>>
>> On 19/08/15 12:49, Renlin Li wrote:
>>> Hi all,
>>>
>>> This simple patch will tighten the conditions when matching movw and
>>> arm_movt rtx pattern.
>>> Those two patterns will generate the following assembly:
>>>
>>> movw w1, #:lower16: dummy + addend
>>> movt w1, #:upper16: dummy + addend
>>>
>>> The addend here is optional. However, it should be an 16-bit signed
>>> value with in the range -32768 <= A <= 32768.
>>>
>>> By impose this restriction explicitly, it will prevent LRA/reload code
>>> from generation invalid high/lo_sum code for arm target.
>>> In process_address_1(), if the address is not legitimate, it will try to
>>> generate high/lo_sum pair to put the address into register. It will
>>> check if the target support those newly generated reload instructions.
>>> By define those two patterns, arm will reject them if conditions is not
>>> meet.
>>>
>>> Otherwise, it might generate movw/movt instructions with addend larger
>>> than 32768, this will cause a GAS error. GAS will produce '''offset out
>>> of range'' error message when the addend for MOVW/MOVT REL relocation is
>>> too large.
>>>
>>>
>>> arm-none-eabi regression tests Okay, Okay to commit to the trunk and
>>> backport to 5.0?

This is ok if it passes an arm bootstrap as well.
Please wait for a few days on trunk for any fallout before backporting
to GCC 5 (you can bootstrap and test the patch there in the meantime).

Thanks,
Kyrill

>>>
>>> Regards,
>>> Renlin
>>>
>>> gcc/ChangeLog:
>>>
>>> 2015-08-19  Renlin Li  <renlin.li@arm.com>
>>>
>>>        * config/arm/arm-protos.h (arm_valid_symbolic_address_p): Declare.
>>>        * config/arm/arm.c (arm_valid_symbolic_address_p): Define.
>>>        * config/arm/arm.md (arm_movt): Use arm_valid_symbolic_address_p.
>>>        * config/arm/constraints.md ("j"): Add check for high code.
>>
>> Is it guaranteed that at this point XEXP (tmp, 0) and XEXP (tmp, 1) are valid?
>> I think before you extract xop0 and xop1 you want to check that tmp is indeed a PLUS
>> and return false if it's not. Only then you should extract XEXP (tmp, 0) and XEXP (tmp, 1).
>>
>>   +  if (GET_CODE (tmp) == PLUS && GET_CODE (xop0) == SYMBOL_REF
>> +      && CONST_INT_P (xop1))
>> +    {
>> +      HOST_WIDE_INT offset = INTVAL (xop1);
>> +      if (offset < -0x8000 || offset > 0x7fff)
>> +	return false;
>> +      else
>> +	return true;
>>
>> I think you can just do "return IN_RANGE (offset, -0x8000, 0x7ffff);"
> Updated accordingly, please check the latest attachment.
>
> Thank you,
> Renlin
>