[PATCH] Fix PR38926, ICE in PRE

Daniel Berlin dberlin@dberlin.org
Wed Jan 28 20:14:00 GMT 2009


Ya, after talking with Richard on IRC, this is basically a missed VN
that PRE happens to discover.
At this point, PRE and VN have already made two value numbers for
these expressions.
The only way to get this to work would be to completely unify the
value numbers when we discover these cases, including the associated
expression lists, into a single VN.
Having only the phi lookup just causes it to get confused about which
value number it's supposed to be using (IE it believes it should be
looking for leaders for Value number 5, and in reality, it knows these
as leaders for Value number 7 because it thought they were different
when we created value numbers).
It's certainly doable (though possibly complex in the middle of PRE
due to set indexing, etc), but it's definitely not worth the trouble
for 4.4

On Wed, Jan 28, 2009 at 11:09 PM, Richard Guenther <rguenther@suse.de> wrote:
> On Tue, 27 Jan 2009, Richard Guenther wrote:
>
>>
>> It happens that during insertion we insert a PHI node which looks the
>> same like an existing PHI node but we insert it for a different
>> value-number.  In this case we may not add the PHI node as expression
>> to that value-number.
>>
>> Bootstrapped and tested on x86_64-unknown-linux-gnu, Danny, does this
>> look ok or do you want to track down why we have this mismatched
>> value-numbers in the first place?  (looks like a missed optimization to
>> me)
>
> On a second thought that whole code is bogus (I added it last year).
> So I have bootstrapped and tested the following instead.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
>
> Richard.
>
> 2009-01-27  Richard Guenther  <rguenther@suse.de>
>
>        PR tree-optimization/38926
>        * tree-ssa-pre.c (add_to_value): Assert we add only expressions
>        with the correct value id to a value.
>        (do_regular_insertion): Use the value number of edoubleprime
>        for the value number of the expr.
>
>        Revert
>        2008-08-21  Richard Guenther  <rguenther@suse.de>
>
>        * tree-ssa-pre.c (insert_into_preds_of_block): Before inserting
>        a PHI ask VN if it is already available.
>        * tree-ssa-sccvn.h (vn_phi_lookup): Declare.
>        * tree-ssa-sccvn.c (vn_phi_lookup): Export.
>
>        * gcc.c-torture/compile/pr38926.c: New testcase.
>
> Index: gcc/tree-ssa-pre.c
> ===================================================================
> *** gcc/tree-ssa-pre.c  (revision 143699)
> --- gcc/tree-ssa-pre.c  (working copy)
> *************** phi_trans_add (pre_expr e, pre_expr v, b
> *** 552,557 ****
> --- 552,560 ----
>  }
>
>
> + static unsigned int
> + get_expr_value_id (pre_expr);
> +
>  /* Add expression E to the expression set of value id V.  */
>
>  void
> *************** add_to_value (unsigned int v, pre_expr e
> *** 559,564 ****
> --- 562,569 ----
>  {
>    bitmap_set_t set;
>
> +   gcc_assert (get_expr_value_id (e) == v);
> +
>    if (v >= VEC_length (bitmap_set_t, value_expressions))
>      {
>        VEC_safe_grow_cleared (bitmap_set_t, heap, value_expressions,
> *************** do_regular_insertion (basic_block block,
> *** 3330,3336 ****
>                          pre_stats.constified++;
>                        }
>                      else
> !                       info->valnum = PRE_EXPR_NAME (edoubleprime);
>                      info->value_id = new_val;
>                    }
>                }
> --- 3340,3346 ----
>                          pre_stats.constified++;
>                        }
>                      else
> !                       info->valnum = VN_INFO (PRE_EXPR_NAME (edoubleprime))->valnum;
>                      info->value_id = new_val;
>                    }
>                }
> Index: gcc/testsuite/gcc.c-torture/compile/pr38926.c
> ===================================================================
> *** gcc/testsuite/gcc.c-torture/compile/pr38926.c       (revision 0)
> --- gcc/testsuite/gcc.c-torture/compile/pr38926.c       (revision 0)
> ***************
> *** 0 ****
> --- 1,41 ----
> + static inline int foo (unsigned _si1)
> + {
> +   if (_si1 != 0)
> +     if (_si1 > 2147483647)
> +       return 1;
> +   return 0;
> + }
> +
> + static inline unsigned bar (unsigned _left, int _right)
> + {
> +   return (unsigned) _right >= 8 ? 1 : _left >> _right;
> + }
> +
> + unsigned g_2;
> + unsigned g_67;
> + volatile unsigned g_162;
> +
> + static inline int func_62 (unsigned p_63)
> + {
> +   p_63 = g_2 & g_67;
> +   if (g_2)
> +     ;
> +   else if (p_63)
> +     return 1;
> +   g_67 = bar (p_63, g_2);
> +   return 0;
> + }
> +
> + unsigned baz (void)
> + {
> +   if (g_2)
> +     for (; g_2 <= -16; g_2 = foo (g_2))
> +       {
> +         for (; g_162; g_162)
> +           func_62 (func_62 (0));
> +         if (g_67)
> +           break;
> +       }
> +   return g_2;
> + }
> +
> Index: gcc/tree-ssa-sccvn.c
> ===================================================================
> *** gcc/tree-ssa-sccvn.c        (revision 143721)
> --- gcc/tree-ssa-sccvn.c        (working copy)
> *************** static VEC(tree, heap) *shared_lookup_ph
> *** 1481,1487 ****
>     value number if it exists in the hash table.  Return NULL_TREE if
>     it does not exist in the hash table. */
>
> ! tree
>  vn_phi_lookup (gimple phi)
>  {
>    void **slot;
> --- 1481,1487 ----
>     value number if it exists in the hash table.  Return NULL_TREE if
>     it does not exist in the hash table. */
>
> ! static tree
>  vn_phi_lookup (gimple phi)
>  {
>    void **slot;
> Index: gcc/tree-ssa-sccvn.h
> ===================================================================
> *** gcc/tree-ssa-sccvn.h        (revision 143721)
> --- gcc/tree-ssa-sccvn.h        (working copy)
> *************** vn_reference_t vn_reference_insert (tree
> *** 184,190 ****
>  vn_reference_t vn_reference_insert_pieces (VEC (tree, gc) *,
>                                           VEC (vn_reference_op_s, heap) *,
>                                           tree, unsigned int);
> - tree vn_phi_lookup (gimple);
>
>  hashval_t vn_nary_op_compute_hash (const vn_nary_op_t);
>  int vn_nary_op_eq (const void *, const void *);
> --- 184,189 ----
> Index: gcc/tree-ssa-pre.c
> ===================================================================
> *** gcc/tree-ssa-pre.c  (revision 143721)
> --- gcc/tree-ssa-pre.c  (working copy)
> *************** insert_into_preds_of_block (basic_block
> *** 2975,2981 ****
>    pre_expr eprime;
>    edge_iterator ei;
>    tree type = get_expr_type (expr);
> !   tree temp, res;
>    gimple phi;
>
>    if (dump_file && (dump_flags & TDF_DETAILS))
> --- 2975,2981 ----
>    pre_expr eprime;
>    edge_iterator ei;
>    tree type = get_expr_type (expr);
> !   tree temp;
>    gimple phi;
>
>    if (dump_file && (dump_flags & TDF_DETAILS))
> *************** insert_into_preds_of_block (basic_block
> *** 3131,3138 ****
>    if (TREE_CODE (type) == COMPLEX_TYPE
>        || TREE_CODE (type) == VECTOR_TYPE)
>      DECL_GIMPLE_REG_P (temp) = 1;
> -
>    phi = create_phi_node (temp, block);
>    FOR_EACH_EDGE (pred, ei, block->preds)
>      {
>        pre_expr ae = avail[pred->src->index];
> --- 3131,3142 ----
>    if (TREE_CODE (type) == COMPLEX_TYPE
>        || TREE_CODE (type) == VECTOR_TYPE)
>      DECL_GIMPLE_REG_P (temp) = 1;
>    phi = create_phi_node (temp, block);
> +
> +   gimple_set_plf (phi, NECESSARY, false);
> +   VN_INFO_GET (gimple_phi_result (phi))->valnum = gimple_phi_result (phi);
> +   VN_INFO (gimple_phi_result (phi))->value_id = val;
> +   VEC_safe_push (gimple, heap, inserted_exprs, phi);
>    FOR_EACH_EDGE (pred, ei, block->preds)
>      {
>        pre_expr ae = avail[pred->src->index];
> *************** insert_into_preds_of_block (basic_block
> *** 3143,3162 ****
>        else
>        add_phi_arg (phi, PRE_EXPR_NAME (avail[pred->src->index]), pred);
>      }
> -   /* If the PHI node is already available, use it.  */
> -   if ((res = vn_phi_lookup (phi)) != NULL_TREE)
> -     {
> -       gimple_stmt_iterator gsi = gsi_for_stmt (phi);
> -       remove_phi_node (&gsi, true);
> -       release_defs (phi);
> -       add_to_value (val, get_or_alloc_expr_for_name (res));
> -       return false;
> -     }
> -
> -   gimple_set_plf (phi, NECESSARY, false);
> -   VN_INFO_GET (gimple_phi_result (phi))->valnum = gimple_phi_result (phi);
> -   VN_INFO (gimple_phi_result (phi))->value_id = val;
> -   VEC_safe_push (gimple, heap, inserted_exprs, phi);
>
>    newphi = get_or_alloc_expr_for_name (gimple_phi_result (phi));
>    add_to_value (val, newphi);
> --- 3147,3152 ----
>



More information about the Gcc-patches mailing list