Re: [PATCH][RFC] Tuplify PRE and some more stuff

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

On Wed, Jul 9, 2008 at 1:35 PM, Richard Guenther <rguenther@suse.de> wrote:
> On Wed, 9 Jul 2008, Daniel Berlin wrote:
>
>> On Wed, Jul 9, 2008 at 12:52 PM, Richard Guenther <rguenther@suse.de>
>> wrote:
>>>
>>> This is a big patch that apart from tuplifying PRE does
>>>
>>>  - allow printing RHS of a stmt only (with a TDF_RHS_ONLY flag hack)
>>>  this reduces the difference between tuples and trunk dump files
>>>  and for example makes all VRP tests magically pass.
>>>  (I don't like that we change testcases on the branch to match what
>>>  we see there -- it is too easy to miss a difference in code this way)
>>>
>>>  - fix some bugs in the SCCVN conversion
>>>
>>>  - adds some gimple helpers and fixes some
>>>
>>>  - re-adds some of the SCCVN insertion code (I'll move that to trunk
>>>  as well)
>>>
>>> the fake store insertion is not converted as it is disabled on the
>>> trunk anyway.
>>>
>>> The patch sofar passes C only bootstrap (all languages bootstrap
>>> dies in building libstdc++ PCH ...), most of the PRE/FRE tests pass but
>>> I didn't yet produce a baseline to compare testresults against.
>>>
>>> Full testing (w/ PCH disabled) still running.
>>>
>>> Should I split the patch?
>>>
>>> Thanks,
>>> Richard.
>>>
>>>
>>> 2008-07-09  Richard Guenther  <rguenther@suse.de>
>>>
>>>       * gimple.h (gimple_assign_ssa_name_copy_p): Declare.
>>>       (gimple_has_lhs): New function.
>>>       * gimple.c (gimple_assign_ssa_name_copy_p): New function.
>>>       * tree-ssa-pre.c: Tuplify.  Enable FRE and PRE.
>>>       (get_or_alloc_expr_for): Handle n-ary expressions.
>>>       * tree-ssa-sccvn.h (copy_reference_ops_from_call): Declare.
>>>       * tree-ssa-sccvn.c (copy_reference_ops_from_call): Export.
>>>       (vn_get_expr_for): Always hand out conversions.
>>>       (visit_reference_op_load): Properly set a value id for
>>>       inserted names.
>>>       (simplify_binary_expression): Use valid_gimple_rhs_p instead of
>>>       valid_gimple_expression_p.
>>>       (simplify_unary_expression): Likewise.
>>>       (set_ssa_val_to): Clear the cached expression.
>>>       * tree-ssa-copy.c (propagate_tree_value_into_stmt): Remove
>>>       redundant gimple_set_location call.
>>>       * gimple-pretty-print.c (print_gimple_stmt): Do not flush if
>>>       TDF_RHS_ONLY.
>>>       (dump_gimple_assign): Dump the RHS as expression if TDF_RHS_ONLY.
>>>       (dump_gimple_call): Likewise.
>>>       (dump_gimple_cond): Likewise.
>>>       * tree-pass.h (TDF_RHS_ONLY): New flag.
>>>
>>>       * testsuite/gcc.dg/tree-ssa/pr25485.c: Revert to trunk version.
>>>
>>> Index: gimple-tuples-branch/gcc/gimple.c
>>> ===================================================================
>>> *** gimple-tuples-branch.orig/gcc/gimple.c      2008-07-09
>>> 10:43:44.000000000 +0200
>>> --- gimple-tuples-branch/gcc/gimple.c   2008-07-09 10:56:13.000000000
>>> +0200
>>> *************** gimple_assign_copy_p (gimple gs)
>>> *** 1838,1843 ****
>>> --- 1838,1857 ----
>>>        && is_gimple_val (gimple_op (gs, 1));
>>>  }
>>>
>>> + + /* Return true if GS is a SSA_NAME copy assignment.  */
>>> + + bool
>>> + gimple_assign_ssa_name_copy_p (gimple gs)
>>> + {
>>> +   return (gimple_code (gs) == GIMPLE_ASSIGN
>>> +         && (get_gimple_rhs_class (gimple_assign_rhs_code (gs))
>>> +             == GIMPLE_SINGLE_RHS)
>>> +         && TREE_CODE (gimple_assign_lhs (gs)) == SSA_NAME
>>> +         && TREE_CODE (gimple_assign_rhs1 (gs)) == SSA_NAME);
>>> + }
>>> + +
>>>  /* Return true if GS is an assignment with a singleton RHS, i.e.,
>>>    there is no operator associated with the assignment itself.
>>>    Unlike gimple_assign_copy_p, this predicate returns true for
>>> Index: gimple-tuples-branch/gcc/gimple.h
>>> ===================================================================
>>> *** gimple-tuples-branch.orig/gcc/gimple.h      2008-07-09
>>> 10:43:44.000000000 +0200
>>> --- gimple-tuples-branch/gcc/gimple.h   2008-07-09 10:56:13.000000000
>>> +0200
>>> *************** void gimple_seq_add_seq (gimple_seq *, g
>>> *** 821,826 ****
>>> --- 821,827 ----
>>>  gimple_seq gimple_seq_copy (gimple_seq);
>>>  int gimple_call_flags (const_gimple);
>>>  bool gimple_assign_copy_p (gimple);
>>> + bool gimple_assign_ssa_name_copy_p (gimple);
>>>  bool gimple_assign_single_p (gimple);
>>>  bool gimple_assign_unary_nop_p (gimple);
>>>  void gimple_set_bb (gimple, struct basic_block_def *);
>>> *************** gimple_call_copy_flags (gimple dest_call
>>> *** 2013,2018 ****
>>> --- 2014,2031 ----
>>>  }
>>>
>>>
>>> + /* Returns true if this is a GIMPLE_ASSIGN or a GIMPLE_CALL with a
>>> +    non-NULL lhs.  */
>>> + + static inline bool
>>> + gimple_has_lhs (gimple stmt)
>>> + {
>>> +   return (is_gimple_assign (stmt)
>>> +         || (is_gimple_call (stmt)
>>> +             && gimple_call_lhs (stmt) != NULL_TREE));
>>> + }
>>> + +
>>>  /* Return the code of the predicate computed by conditional statement
>>> GS.
>>>  */
>>>
>>>  static inline enum tree_code
>>> Index: gimple-tuples-branch/gcc/tree-ssa-pre.c
>>> ===================================================================
>>> *** gimple-tuples-branch.orig/gcc/tree-ssa-pre.c        2008-07-09
>>> 10:43:44.000000000 +0200
>>> --- gimple-tuples-branch/gcc/tree-ssa-pre.c     2008-07-09
>>> 17:46:28.000000000 +0200
>>> *************** along with GCC; see the file COPYING3.
>>> *** 48,55 ****
>>>  #include "params.h"
>>>  #include "dbgcnt.h"
>>>
>>> - /* FIXME tuples.  */
>>> - #if 0
>>>  /* TODO:
>>>
>>>    1. Avail sets can be shared by making an avail_find_leader that
>>> --- 48,53 ----
>>> *************** static struct
>>> *** 424,430 ****
>>>  } pre_stats;
>>>
>>>  static bool do_partial_partial;
>>> ! static pre_expr bitmap_find_leader (bitmap_set_t, unsigned int , tree);
>>>  static void bitmap_value_insert_into_set (bitmap_set_t, pre_expr);
>>>  static void bitmap_value_replace_in_set (bitmap_set_t, pre_expr);
>>>  static void bitmap_set_copy (bitmap_set_t, bitmap_set_t);
>>> --- 422,428 ----
>>>  } pre_stats;
>>>
>>>  static bool do_partial_partial;
>>> ! static pre_expr bitmap_find_leader (bitmap_set_t, unsigned int,
>>> gimple);
>>>  static void bitmap_value_insert_into_set (bitmap_set_t, pre_expr);
>>>  static void bitmap_value_replace_in_set (bitmap_set_t, pre_expr);
>>>  static void bitmap_set_copy (bitmap_set_t, bitmap_set_t);
>>> *************** static bool bitmap_set_contains_value (b
>>> *** 432,440 ****
>>>  static void bitmap_insert_into_set (bitmap_set_t, pre_expr);
>>>  static void bitmap_insert_into_set_1 (bitmap_set_t, pre_expr, bool);
>>>  static bitmap_set_t bitmap_set_new (void);
>>> ! static tree create_expression_by_pieces (basic_block, pre_expr, tree,
>>> tree,
>>> !                                        tree);
>>> ! static tree find_or_generate_expression (basic_block, pre_expr, tree,
>>> tree);
>>>
>>>  /* We can add and remove elements and entries to and from sets
>>>    and hash tables, so we use alloc pools for them.  */
>>> --- 430,439 ----
>>>  static void bitmap_insert_into_set (bitmap_set_t, pre_expr);
>>>  static void bitmap_insert_into_set_1 (bitmap_set_t, pre_expr, bool);
>>>  static bitmap_set_t bitmap_set_new (void);
>>> ! static tree create_expression_by_pieces (basic_block, pre_expr,
>>> gimple_seq
>>> *,
>>> !                                        gimple, tree);
>>> ! static tree find_or_generate_expression (basic_block, pre_expr,
>>> gimple_seq
>>> *,
>>> !                                        gimple);
>>>
>>>  /* We can add and remove elements and entries to and from sets
>>>    and hash tables, so we use alloc pools for them.  */
>>> *************** get_or_alloc_expr_for (tree t)
>>> *** 1038,1043 ****
>>> --- 1037,1057 ----
>>>     return get_or_alloc_expr_for_name (t);
>>>   else if (is_gimple_min_invariant (t))
>>>     return get_or_alloc_expr_for_constant (t);
>>> +   else if (UNARY_CLASS_P (t)
>>> +          || BINARY_CLASS_P (t)
>>> +          || COMPARISON_CLASS_P (t)
>>> +          || TREE_CODE (t) == ADDR_EXPR)
>>> +     {
>>> +       vn_nary_op_t result;
>>> +       if (vn_nary_op_lookup (t, &result) != NULL_TREE)
>>> +       {
>>> +         pre_expr e = (pre_expr) pool_alloc (pre_expr_pool);
>>> +         e->kind = NARY;
>>> +         e->u.nary = result;
>>> +         alloc_expression_id (e);
>>> +         return e;
>>> +       }
>>> +     }
>>>   return NULL;
>>
>>
>> vn_nary_op_lookup can return a NULL result but still fill in the
>> argument you pass in.
>> This happens, e.g. when values are created during phi translation and
>> inserted into the table.
>>
>> Have you hit a case where it is actually necessary to add the code above?
>> If so, i'd love to see it :)
>
> This fixes for example gcc.dg/tree-ssa/ssa-fre-13.c and -14.c which
> is the SCCVN insertion stuff.  We end up with (void *) &a for the
> expression.  It doesn't yet fix the VIEW_CONVERT_EXPR stuff (but
> probably because that is tcc_reference, which needs to be handled
> similarly above).
>
> So, this tries to address do_SCCVN_insertion
>
>

Okay.
In that case, check whether the variable you call result is null,
instead of the return value, since you don't care about whether it has
an SSA_NAME associated.