On Wed, May 2, 2012 at 3:14 AM, Michael Matz <[email protected]> wrote:
> Hi,
>
> this introduces a new helper (gsi_replace_with_seq) which can replace a
> single statement with a sequence, and makes use of it in
> gimplify_and_update_call_from_tree. This make sure that the statements
> aren't inserted into the target sequence while they still are in the
> original one. Could also be solved by gsi_removing them just before
> inserting them, but this way seems much nicer.
>
> The tree-ssa-forwprop.c hunk ensures that when we remove the statement
> that the iterator (in the caller) pointing to it isn't invalidated. I
> think this was from earlier experiments of how to represent the data
> structures and might not be strictly needed anymore, but I've always
> tested with it, and it's definitely not harmful.
>
> As per [0/6] regstrapped with the other five on x86_64-linux. Okay for
> trunk?
>
>
> Ciao,
> Michael.
> ----------------------
> 2012-05-02 Michael Matz <[email protected]>
>
> * gimple-fold.c (gimplify_and_update_call_from_tree): Use
> gsi_replace_with_seq, instead of inserting itself.
> * gimple-iterator.c (gsi_replace_with_seq): New function.
> * tree-ssa-forwprop.c (forward_propagate_comparison): Take
> iterator instead of statement, advance it.
> (ssa_forward_propagate_and_combine): Adjust call to above.
>
> Index: gimple-fold.c
> ===================================================================
> *** gimple-fold.c.orig 2012-05-01 22:44:02.000000000 +0200
> --- gimple-fold.c 2012-05-01 22:44:06.000000000 +0200
> *************** gimplify_and_update_call_from_tree (gimp
> *** 551,557 ****
> gimple_stmt_iterator i;
> gimple_seq stmts = NULL;
> struct gimplify_ctx gctx;
> - gimple last;
> gimple laststore;
> tree reaching_vuse;
>
> --- 551,556 ----
> *************** gimplify_and_update_call_from_tree (gimp
> *** 620,636 ****
>
> /* Second iterate over the statements forward, assigning virtual
> operands to their uses. */
> - last = NULL;
> reaching_vuse = gimple_vuse (stmt);
> for (i = gsi_start (stmts); !gsi_end_p (i); gsi_next (&i))
> {
> - /* Do not insert the last stmt in this loop but remember it
> - for replacing the original statement. */
> - if (last)
> - {
> - gsi_insert_before (si_p, last, GSI_NEW_STMT);
> - gsi_next (si_p);
> - }
> new_stmt = gsi_stmt (i);
> /* The replacement can expose previously unreferenced variables. */
> if (gimple_in_ssa_p (cfun))
> --- 619,627 ----
> *************** gimplify_and_update_call_from_tree (gimp
> *** 642,648 ****
> gimple_set_modified (new_stmt, true);
> if (gimple_vdef (new_stmt))
> reaching_vuse = gimple_vdef (new_stmt);
> - last = new_stmt;
> }
>
> /* If the new sequence does not do a store release the virtual
> --- 633,638 ----
> *************** gimplify_and_update_call_from_tree (gimp
> *** 659,666 ****
> }
> }
>
> ! /* Finally replace rhe original statement with the last. */
> ! gsi_replace (si_p, last, false);
> }
>
> /* Return the string length, maximum string length or maximum value of
> --- 649,656 ----
> }
> }
>
> ! /* Finally replace the original statement with the sequence. */
> ! gsi_replace_with_seq (si_p, stmts, false);
> }
>
> /* Return the string length, maximum string length or maximum value of
> Index: gimple-iterator.c
> ===================================================================
> *** gimple-iterator.c.orig 2012-05-01 22:43:34.000000000 +0200
> --- gimple-iterator.c 2012-05-01 22:44:06.000000000 +0200
> *************** gsi_replace (gimple_stmt_iterator *gsi,
> *** 433,438 ****
> --- 433,456 ----
> update_modified_stmt (stmt);
> }
>
> + void
> + gsi_replace_with_seq (gimple_stmt_iterator *gsi, gimple_seq seq,
> + bool update_eh_info)
Ok with adding a proper function comment for this.
Thanks,
Richard.
> + {
> + gimple_stmt_iterator seqi;
> + gimple last;
> + if (gimple_seq_empty_p (seq))
> + {
> + gsi_remove (gsi, true);
> + return;
> + }
> + seqi = gsi_last (seq);
> + last = gsi_stmt (seqi);
> + gsi_remove (&seqi, false);
> + gsi_insert_seq_before (gsi, seq, GSI_SAME_STMT);
> + gsi_replace (gsi, last, update_eh_info);
> + }
> +
>
> /* Insert statement STMT before the statement pointed-to by iterator I.
> M specifies how to update iterator I after insertion (see enum
> Index: tree-ssa-forwprop.c
> ===================================================================
> *** tree-ssa-forwprop.c.orig 2012-05-01 22:39:07.000000000 +0200
> --- tree-ssa-forwprop.c 2012-05-01 22:44:06.000000000 +0200
> *************** forward_propagate_addr_expr (tree name,
> *** 1202,1217 ****
> }
>
>
> ! /* Forward propagate the comparison defined in STMT like
> cond_1 = x CMP y to uses of the form
> a_1 = (T')cond_1
> a_1 = !cond_1
> a_1 = cond_1 != 0
> ! Returns true if stmt is now unused. */
>
> static bool
> ! forward_propagate_comparison (gimple stmt)
> {
> tree name = gimple_assign_lhs (stmt);
> gimple use_stmt;
> tree tmp = NULL_TREE;
> --- 1202,1219 ----
> }
>
>
> ! /* Forward propagate the comparison defined in *DEFGSI like
> cond_1 = x CMP y to uses of the form
> a_1 = (T')cond_1
> a_1 = !cond_1
> a_1 = cond_1 != 0
> ! Returns true if stmt is now unused. Advance DEFGSI to the next
> ! statement. */
>
> static bool
> ! forward_propagate_comparison (gimple_stmt_iterator *defgsi)
> {
> + gimple stmt = gsi_stmt (*defgsi);
> tree name = gimple_assign_lhs (stmt);
> gimple use_stmt;
> tree tmp = NULL_TREE;
> *************** forward_propagate_comparison (gimple stm
> *** 1224,1241 ****
> && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (gimple_assign_rhs1 (stmt)))
> || (TREE_CODE (gimple_assign_rhs2 (stmt)) == SSA_NAME
> && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (gimple_assign_rhs2 (stmt))))
> ! return false;
>
> /* Do not un-cse comparisons. But propagate through copies. */
> use_stmt = get_prop_dest_stmt (name, &name);
> if (!use_stmt
> || !is_gimple_assign (use_stmt))
> ! return false;
>
> code = gimple_assign_rhs_code (use_stmt);
> lhs = gimple_assign_lhs (use_stmt);
> if (!INTEGRAL_TYPE_P (TREE_TYPE (lhs)))
> ! return false;
>
> /* We can propagate the condition into a statement that
> computes the logical negation of the comparison result. */
> --- 1226,1243 ----
> && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (gimple_assign_rhs1 (stmt)))
> || (TREE_CODE (gimple_assign_rhs2 (stmt)) == SSA_NAME
> && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (gimple_assign_rhs2 (stmt))))
> ! goto bailout;
>
> /* Do not un-cse comparisons. But propagate through copies. */
> use_stmt = get_prop_dest_stmt (name, &name);
> if (!use_stmt
> || !is_gimple_assign (use_stmt))
> ! goto bailout;
>
> code = gimple_assign_rhs_code (use_stmt);
> lhs = gimple_assign_lhs (use_stmt);
> if (!INTEGRAL_TYPE_P (TREE_TYPE (lhs)))
> ! goto bailout;
>
> /* We can propagate the condition into a statement that
> computes the logical negation of the comparison result. */
> *************** forward_propagate_comparison (gimple stm
> *** 1249,1261 ****
> enum tree_code inv_code;
> inv_code = invert_tree_comparison (gimple_assign_rhs_code (stmt),
> nans);
> if (inv_code == ERROR_MARK)
> ! return false;
>
> tmp = build2 (inv_code, TREE_TYPE (lhs), gimple_assign_rhs1 (stmt),
> gimple_assign_rhs2 (stmt));
> }
> else
> ! return false;
>
> gsi = gsi_for_stmt (use_stmt);
> gimple_assign_set_rhs_from_tree (&gsi, unshare_expr (tmp));
> --- 1251,1263 ----
> enum tree_code inv_code;
> inv_code = invert_tree_comparison (gimple_assign_rhs_code (stmt),
> nans);
> if (inv_code == ERROR_MARK)
> ! goto bailout;
>
> tmp = build2 (inv_code, TREE_TYPE (lhs), gimple_assign_rhs1 (stmt),
> gimple_assign_rhs2 (stmt));
> }
> else
> ! goto bailout;
>
> gsi = gsi_for_stmt (use_stmt);
> gimple_assign_set_rhs_from_tree (&gsi, unshare_expr (tmp));
> *************** forward_propagate_comparison (gimple stm
> *** 1271,1278 ****
> --- 1273,1288 ----
> fprintf (dump_file, "'\n");
> }
>
> + /* When we remove stmt now the iterator defgsi goes off it's current
> + sequence, hence advance it now. */
> + gsi_next (defgsi);
> +
> /* Remove defining statements. */
> return remove_prop_source_from_use (name);
> +
> + bailout:
> + gsi_next (defgsi);
> + return false;
> }
>
>
> *************** ssa_forward_propagate_and_combine (void)
> *** 2682,2690 ****
> }
> else if (TREE_CODE_CLASS (code) == tcc_comparison)
> {
> ! if (forward_propagate_comparison (stmt))
> cfg_changed = true;
> - gsi_next (&gsi);
> }
> else
> gsi_next (&gsi);
> --- 2692,2699 ----
> }
> else if (TREE_CODE_CLASS (code) == tcc_comparison)
> {
> ! if (forward_propagate_comparison (&gsi))
> cfg_changed = true;
> }
> else
> gsi_next (&gsi);
