[Bug target/113255] [11/12/13 Regression] wrong code with -O2 -mtune=k8

["rguenth at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113255

--- Comment #15 from Richard Biener <rguenth at gcc dot gnu.org> ---
The issue is also that via CSELIB we go from the good

(minus:DI (reg/f:DI 119)
    (reg:DI 115))

to

(minus:DI (value:DI 11:11 @0x41fca00/0x41ec410)
    (value:DI 10:15448 @0x41fc9e8/0x41ec3e0))

and later when DSE does cselib_expand_value_rtx on the value it produces

(minus:DI (reg/f:DI 119)
    (minus:DI (reg/f:DI 120)
        (reg/f:DI 114)))

which simplify_rtx then turns into

(minus:DI (plus:DI (reg/f:DI 114)
        (reg/f:DI 119))
    (reg/f:DI 120))

note how that associates things in a way that confuses us later.  In particular
the loc for (value:DI 10:15448) (aka the inner minus) isn't REG_POINTER
(after you fix i386 RTL expansion) but after the re-assloc there's only
the wrong REG_POINTER immediately visible.

DSE gets this all back-and-forth into/out-of CSELIB, it feels a bit of a mess.
It obviously relies on the expansion to discover base values.

First the x86 backend should avoid having a REG_POINTER as the pointer
difference:

diff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc
index 0d817fc3f3b..26c48e8b0c8 100644
--- a/gcc/config/i386/i386-expand.cc
+++ b/gcc/config/i386/i386-expand.cc
@@ -8090,7 +8090,7 @@
expand_set_or_cpymem_prologue_epilogue_by_misaligned_moves (rtx destmem, rtx
src
       /* See how many bytes we skipped.  */
       saveddest = expand_simple_binop (GET_MODE (*destptr), MINUS, saveddest,
                                       *destptr,
-                                      saveddest, 1, OPTAB_DIRECT);
+                                      NULL_RTX, 1, OPTAB_DIRECT);
       /* Adjust srcptr and count.  */
       if (!issetmem)
        *srcptr = expand_simple_binop (GET_MODE (*srcptr), MINUS, *srcptr,

We can avoid the issue by avoiding re-association of pointer MINUS:

diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
index ee75079917f..0108d0aa3bd 100644
--- a/gcc/simplify-rtx.cc
+++ b/gcc/simplify-rtx.cc
@@ -3195,11 +3195,15 @@ simplify_context::simplify_binary_operation_1 (rtx_code
code,
          canonicalize (minus A (plus B C)) to (minus (minus A B) C).
         Don't use the associative law for floating point.
         The inaccuracy makes it nonassociative,
-        and subtle programs can break if operations are associated.  */
+        and subtle programs can break if operations are associated.
+        Don't use the associative law when subtracting a MINUS from
+        a REG_POINTER as that can trick find_base_term into discovering
+        the wrong base.  */

       if (INTEGRAL_MODE_P (mode)
          && (plus_minus_operand_p (op0)
-             || plus_minus_operand_p (op1))
+             || ((!REG_P (op0) || !REG_POINTER (op0))
+                 && plus_minus_operand_p (op1)))
          && (tem = simplify_plus_minus (code, mode, op0, op1)) != 0)
        return tem;


or we can avoid it with a more dangerous (IMHO) "fix" like the following
which while it looks good on the front, isn't reliable and might instead
trick find_base_term to deflect to another invalid base.

diff --git a/gcc/alias.cc b/gcc/alias.cc
index 3672bf277b9..f589a1fa47a 100644
--- a/gcc/alias.cc
+++ b/gcc/alias.cc
@@ -2094,7 +2101,14 @@ find_base_term (rtx x, vec<std::pair<cselib_val *,
        if (base != NULL_RTX
            && ((REG_P (tmp1) && REG_POINTER (tmp1))
                 || known_base_value_p (base)))
-         return base;
+         {
+           /* If, for a MINUS, we find another base value in the
+              other operand, fail.  */
+           if (GET_CODE (x) == MINUS
+               && find_base_term (tmp2, visited_vals) != NULL)
+             return 0;
+           return base;
+         }
        base = find_base_term (tmp2, visited_vals);
        if (base != NULL_RTX
            && ((REG_P (tmp2) && REG_POINTER (tmp2))

This all shows alternatives that might be suitable for branches and possibly
trunk when we decide to revert the fix that's currently there.