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

login
register
mail settings
Submitter Richard Guenther
Date April 4, 2013, 2:18 p.m.
Message ID <CAFiYyc1AC_sUAGrTmaS2rinBX87CfFstcAf9WnzyP24RxtmScQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/233838/
State New
Headers show

Comments

Richard Guenther - April 4, 2013, 2:18 p.m.
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.

Patch

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