FW: [PATCH] Avoid a few find_base_term calls in alias.c

[Richard Biener]

On Tue, Apr 9, 2013 at 9:42 AM, Igor Zamyatin <izamyatin@gmail.com> wrote:
> Richard,
>
> Any plans to commit your new fix?

I did already.

Richard.

>
> Thanks,
> Igor
>
>> On Thu, Apr 4, 2013 at 3:33 PM, Yuri Rumyantsev <ysrumyan@gmail.com>
>> wrote:
>> > Hi Richard,
>> >
>> > You slightly change behavior of find_base_term, namely before your fix
>> > we have the following code:
>> >
>> > if (REG_P (tmp1) && REG_POINTER (tmp1))  {
>> >    rtx base = find_base_term (tmp1);
>> >    if (base)
>> >      return base;
>> >  }
>> >
>> > but in your fix you added the following checks on base:
>> >
>> > tmp1 = find_base_term (tmp1);
>> > if (tmp1 != NULL_RTX
>> >    && ((REG_P (tmp1) && REG_POINTER (tmp1))
>> > || known_base_value_p (tmp1)))
>> >  return tmp1;
>> > Why you added these checks since function arguments may not considered
>> > as bases.
>>
>> If you continue down the original function there is (quoted fully)
>>
>>         /* If either operand is known to be a pointer, then use it
>>            to determine the base term.  */
>>         if (REG_P (tmp1) && REG_POINTER (tmp1))
>>           {
>>             rtx base = find_base_term (tmp1);
>>             if (base)
>>               return base;
>>           }
>>
>>         if (REG_P (tmp2) && REG_POINTER (tmp2))
>>           {
>>             rtx base = find_base_term (tmp2);
>>             if (base)
>>               return base;
>>           }
>>
>>         /* Neither operand was known to be a pointer.  Go ahead and find
>> the
>>            base term for both operands.  */
>>         tmp1 = find_base_term (tmp1);
>>         tmp2 = find_base_term (tmp2);
>>
>> so we call find_base_term on tmp1 anyway, and after the modification
>>
>>         tmp1 = find_base_term (tmp1);
>>         if (tmp1 != NULL_RTX
>>             && ((REG_P (tmp1) && REG_POINTER (tmp1))
>>                  || known_base_value_p (tmp1)))
>>           return tmp1;
>>
>> we take it if it is non-NULL and ... ah, I see.  We now check
>> REG_P/REG_POINTER on the new value.  That was indeed unintended.
>>
>> I'll fix it like below.
>>
>> Richard.
>>
>> 2013-04-04  Richard Biener  <rguenther@suse.de>
>>
>>         * alias.c (find_base_term): Fix thinko in previous change.
>>
>> Index: gcc/alias.c
>> ===================================================================
>> --- gcc/alias.c (revision 197480)
>> +++ gcc/alias.c (working copy)
>> @@ -1687,16 +1687,16 @@ find_base_term (rtx x)
>>            term is from a pointer or is a named object or a special
>> address
>>            (like an argument or stack reference), then use it for the
>>            base term.  */
>> -       tmp1 = find_base_term (tmp1);
>> -       if (tmp1 != NULL_RTX
>> +       rtx base = find_base_term (tmp1);
>> +       if (base != NULL_RTX
>>             && ((REG_P (tmp1) && REG_POINTER (tmp1))
>> -                || known_base_value_p (tmp1)))
>> -         return tmp1;
>> -       tmp2 = find_base_term (tmp2);
>> -       if (tmp2 != NULL_RTX
>> +                || known_base_value_p (base)))
>> +         return base;
>> +       base = find_base_term (tmp2);
>> +       if (base != NULL_RTX
>>             && ((REG_P (tmp2) && REG_POINTER (tmp2))
>> -                || known_base_value_p (tmp2)))
>> -         return tmp2;
>> +                || known_base_value_p (base)))
>> +         return base;
>>
>>         /* We could not determine which of the two operands was the
>>            base register and which was the index.  So we can determine
>
>