Re: [PATCH] Fix PR37258

["Daniel Berlin" <dberlin at dberlin dot org>]

On Thu, Sep 18, 2008 at 11:25 AM, Richard Guenther <rguenther@suse.de> wrote:
>
> This fixes PR37258 where PRE ends up creating an expression with
> mismatched types because SCCVN decides to value number two PHIs the
> same even though they have different incompatible arguments.


> The fix is similar to what we did for constant value ids.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
>

Looks fine to me.
(I do find it a bit sad that we can't just treat constant bits as bits
and have things work okay in the end without a whole mess of crap, but
c'est la vie)

> Richard.
>
> 2008-09-18  Richard Guenther  <rguenther@suse.de>
>
>        PR tree-optimization/37258
>        * tree-ssa-sccvn.c (vn_phi_compute_hash): Include the precision
>        and signedness for integral types.
>        (vn_phi_eq): Require compatible types.
>
>        * gcc.c-torture/compile/pr37258.c: New testcase.
>
> Index: gcc/tree-ssa-sccvn.c
> ===================================================================
> *** gcc/tree-ssa-sccvn.c        (revision 140452)
> --- gcc/tree-ssa-sccvn.c        (working copy)
> *************** vn_phi_compute_hash (vn_phi_t vp1)
> *** 1402,1410 ****
> --- 1402,1418 ----
>    hashval_t result = 0;
>    int i;
>    tree phi1op;
> +   tree type;
>
>    result = vp1->block->index;
>
> +   /* If all PHI arguments are constants we need to distinguish
> +      the PHI node via its type.  */
> +   type = TREE_TYPE (VEC_index (tree, vp1->phiargs, 0));
> +   result += (INTEGRAL_TYPE_P (type)
> +            + (INTEGRAL_TYPE_P (type)
> +               ? TYPE_PRECISION (type) + TYPE_UNSIGNED (type) : 0));
> +
>    for (i = 0; VEC_iterate (tree, vp1->phiargs, i, phi1op); i++)
>      {
>        if (phi1op == VN_TOP)
> *************** vn_phi_eq (const void *p1, const void *p
> *** 1437,1442 ****
> --- 1445,1456 ----
>        int i;
>        tree phi1op;
>
> +       /* If the PHI nodes do not have compatible types
> +        they are not the same.  */
> +       if (!types_compatible_p (TREE_TYPE (VEC_index (tree, vp1->phiargs, 0)),
> +                              TREE_TYPE (VEC_index (tree, vp2->phiargs, 0))))
> +       return false;
> +
>        /* Any phi in the same block will have it's arguments in the
>         same edge order, because of how we store phi nodes.  */
>        for (i = 0; VEC_iterate (tree, vp1->phiargs, i, phi1op); i++)
> Index: gcc/testsuite/gcc.c-torture/compile/pr37258.c
> ===================================================================
> *** gcc/testsuite/gcc.c-torture/compile/pr37258.c       (revision 0)
> --- gcc/testsuite/gcc.c-torture/compile/pr37258.c       (revision 0)
> ***************
> *** 0 ****
> --- 1,58 ----
> + typedef signed char int8_t;
> + typedef short int int16_t;
> + typedef int int32_t;
> + __extension__ typedef long long int int64_t;
> + typedef unsigned short int uint16_t;
> + typedef unsigned int uint32_t;
> + static inline unsigned int
> + lshift_u_s (unsigned int left, int right)
> + {
> +   if ((right) || (right >= sizeof (unsigned int) * 8)
> +       || (left > (4294967295U >> right)))
> +     return left;
> + }
> + static inline unsigned long int
> + div_rhs (long int rhs)
> + {
> +   if (rhs == 0)
> +     return 1;
> +   return rhs;
> + }
> +
> + uint32_t g_230;
> + int8_t g_294;
> + uint16_t g_316;
> + uint32_t g_334;
> + int32_t g_375;
> + int64_t g_380;
> + int32_t func_99 (int16_t p_100, int32_t p_101, int32_t p_102, int32_t p_103,
> +                  int64_t p_105, int32_t p_106, int32_t p_107, int16_t p_108,
> +                  int16_t p_109);
> + int32_t
> + func_77 (int64_t p_79)
> + {
> +   int16_t l_397;
> +   if (mod_rhs (p_79))
> +     p_79 = 1;
> +   else
> +     for (p_79 = 0; 0; p_79 += 1)
> +       {
> +       }
> +   if (lshift_s_s (1, func_112 (2L, (lshift_u_s (g_334, p_79)))))
> +     {
> +       int8_t l_384;
> +       int64_t l_414;
> +       if (lshift_u_s (g_375, 1))
> +         {
> +           func_23 (func_99 (1, 1, 1, 1, g_230, p_79, 1, g_334, 1), 1);
> +           for (p_79 = 0; 0; ++p_79)
> +             {
> +             }
> +         }
> +       if (div_rhs (func_82 (1, 1, g_380, 1, l_397, 1, 1)))
> +         func_99 ((func_82
> +                   (1, g_334, g_294, func_112 (1, (p_79 & 1)), g_316, 1, 1)),
> +                  1, (0xFBA25CA382A8CA74LL), l_384, l_414, 0L, 1, 1, 1);
> +     }
> + }
> +
>