Re: RFC: patch to build GCC for arm with LRA

[Yvan Roux <yvan dot roux at linaro dot org>]

Yes indeed ! here is a fixed patch.

In strip_address_mutations we now have 3 if/else if statements with
the same body which could be factorized in:

      if ((GET_RTX_CLASS (code) == RTX_UNARY)
          /* Things like SIGN_EXTEND, ZERO_EXTEND and TRUNCATE can be
             used to convert between pointer sizes.  */
          || (lsb_bitfield_op_p (*loc))
          /* Bitfield operations [SIGN|ZERO]_EXTRACT from the least significant
             bit can be used too.  */
          || (code == AND && CONST_INT_P (XEXP (*loc, 1))))
          /* (and ... (const_int -X)) is used to align to X bytes.  */
        loc = &XEXP (*loc, 0);

if you think that it doesn't affect too much the readability.

Many Thanks,
Yvan

On 11 September 2013 09:32, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> Yvan Roux <yvan.roux@linaro.org> writes:
>> @@ -5454,6 +5454,16 @@ strip_address_mutations (rtx *loc, enum rtx_code *outer_code)
>>       /* Things like SIGN_EXTEND, ZERO_EXTEND and TRUNCATE can be
>>          used to convert between pointer sizes.  */
>>       loc = &XEXP (*loc, 0);
>> +      else if (GET_RTX_CLASS (code) == RTX_BITFIELD_OPS)
>> +     {
>> +       /* Bitfield operations [SIGN|ZERO]_EXTRACT can be used too.  */
>> +       enum machine_mode mode = GET_MODE(*loc);
>> +       unsigned HOST_WIDE_INT len = INTVAL (XEXP (*loc, 1));
>> +       HOST_WIDE_INT pos = INTVAL (XEXP (*loc, 2));
>> +
>> +       if (pos == (BITS_BIG_ENDIAN ? GET_MODE_PRECISION (mode) - len : 0))
>> +         loc = &XEXP (*loc, 0);
>> +     }
>
> This means that the other values of "pos" bypass the:
>
>       else
>         return loc;
>
> so you'll get an infinite loop.  I think it would be neater to split
> this out into:
>
> /* Return true if X is a sign_extract or zero_extract from the least
>    significant bit.  */
>
> static bool
> lsb_bitfield_op_p (rtx X)
> {
>   ...;
> }
>
>     else if (lsb_bitfield_op_p (*loc))
>       loc = &XEXP (*loc, 0);
>
> Looks good to me otherwise FWIW.
>
> Thanks,
> Richard

Attachment: arm-lra.patch
Description: Binary data