On Wed, May 2, 2012 at 3:14 AM, Michael Matz <m...@suse.de> 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 <m...@suse.de> > > * 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);