On Thu, Sep 18, 2025 at 10:49 PM Andrew Pinski <[email protected]> wrote: > > On Tue, Sep 16, 2025 at 11:50 PM Richard Biener > <[email protected]> wrote: > > > > On Wed, Sep 17, 2025 at 4:55 AM Andrew Pinski > > <[email protected]> wrote: > > > > > > Since now optimize_aggr_zeroprop and optimize_agr_copyprop work by > > > forward walk to prop > > > the zero/aggregate and does not change the statement at hand, there is no > > > reason to > > > repeat the loop if they do anything. This will prevent problems like PR > > > 121962 from > > > happening again as we will never loop. > > > > > > Bootstrapped and tested on x86_64-linux-gnu. > > > > > > gcc/ChangeLog: > > > > > > * tree-ssa-forwprop.cc (optimize_aggr_zeroprop_1): Change return > > > type > > > to void. > > > (optimize_aggr_zeroprop): Likewise. > > > (optimize_agr_copyprop_1): Likewise. > > > (optimize_agr_copyprop_arg): Likewise. > > > (optimize_agr_copyprop): Likewise. > > > (simplify_builtin_call): Handle update of the return type > > > of optimize_aggr_zeroprop. > > > (pass_forwprop::execute): Likewise and optimize_agr_copyprop. > > > > > > Signed-off-by: Andrew Pinski <[email protected]> > > > --- > > > gcc/tree-ssa-forwprop.cc | 90 ++++++++++++++++------------------------ > > > 1 file changed, 35 insertions(+), 55 deletions(-) > > > > > > diff --git a/gcc/tree-ssa-forwprop.cc b/gcc/tree-ssa-forwprop.cc > > > index 833a354ce2c..357eec54d00 100644 > > > --- a/gcc/tree-ssa-forwprop.cc > > > +++ b/gcc/tree-ssa-forwprop.cc > > > @@ -1190,7 +1190,7 @@ constant_pointer_difference (tree p1, tree p2) > > > /* Helper function for optimize_aggr_zeroprop. > > > Props the zeroing (memset, VAL) that was done in DEST+OFFSET:LEN > > > (DEFSTMT) into the STMT. Returns true if the STMT was updated. */ > > > -static bool > > > +static void > > > optimize_aggr_zeroprop_1 (gimple *defstmt, gimple *stmt, > > > tree dest, poly_int64 offset, tree val, > > > poly_offset_int len) > > > @@ -1214,29 +1214,29 @@ optimize_aggr_zeroprop_1 (gimple *defstmt, gimple > > > *stmt, > > > : TYPE_SIZE_UNIT (TREE_TYPE (src2))); > > > /* Can only handle zero memsets. */ > > > if (!integer_zerop (val)) > > > - return false; > > > + return; > > > } > > > else > > > - return false; > > > + return; > > > > > > if (len2 == NULL_TREE > > > || !poly_int_tree_p (len2)) > > > - return false; > > > + return; > > > > > > src2 = get_addr_base_and_unit_offset (src2, &offset2); > > > if (src2 == NULL_TREE > > > || maybe_lt (offset2, offset)) > > > - return false; > > > + return; > > > > > > if (!operand_equal_p (dest, src2, 0)) > > > - return false; > > > + return; > > > > > > /* [ dest + offset, dest + offset + len - 1 ] is set to val. > > > Make sure that > > > [ dest + offset2, dest + offset2 + len2 - 1 ] is a subset of that. > > > */ > > > if (maybe_gt (wi::to_poly_offset (len2) + (offset2 - offset), > > > len)) > > > - return false; > > > + return; > > > > > > if (dump_file && (dump_flags & TDF_DETAILS)) > > > { > > > @@ -1282,8 +1282,6 @@ optimize_aggr_zeroprop_1 (gimple *defstmt, gimple > > > *stmt, > > > /* Mark the bb for eh cleanup if needed. */ > > > if (maybe_clean_or_replace_eh_stmt (orig_stmt, stmt)) > > > bitmap_set_bit (to_purge, gimple_bb (stmt)->index); > > > - > > > - return true; > > > } > > > > > > /* Optimize > > > @@ -1295,19 +1293,18 @@ optimize_aggr_zeroprop_1 (gimple *defstmt, gimple > > > *stmt, > > > Similarly for memset (&a, ..., sizeof (a)); instead of a = {}; > > > and/or memcpy (&b, &a, sizeof (a)); instead of b = a; */ > > > > > > -static bool > > > +static void > > > optimize_aggr_zeroprop (gimple_stmt_iterator *gsip, bool full_walk) > > > { > > > ao_ref read; > > > gimple *stmt = gsi_stmt (*gsip); > > > if (gimple_has_volatile_ops (stmt)) > > > - return false; > > > + return; > > > > > > tree dest = NULL_TREE; > > > tree val = integer_zero_node; > > > tree len = NULL_TREE; > > > bool can_use_tbba = true; > > > - bool changed = false; > > > > > > if (gimple_call_builtin_p (stmt, BUILT_IN_MEMSET) > > > && TREE_CODE (gimple_call_arg (stmt, 0)) == ADDR_EXPR > > > @@ -1362,7 +1359,7 @@ optimize_aggr_zeroprop (gimple_stmt_iterator *gsip, > > > bool full_walk) > > > } > > > > > > if (dest == NULL_TREE) > > > - return false; > > > + return; > > > > > > if (len == NULL_TREE) > > > len = (TREE_CODE (dest) == COMPONENT_REF > > > @@ -1370,13 +1367,13 @@ optimize_aggr_zeroprop (gimple_stmt_iterator > > > *gsip, bool full_walk) > > > : TYPE_SIZE_UNIT (TREE_TYPE (dest))); > > > if (len == NULL_TREE > > > || !poly_int_tree_p (len)) > > > - return false; > > > + return; > > > > > > /* This store needs to be on the byte boundary and pointing to an > > > object. */ > > > poly_int64 offset; > > > tree dest_base = get_addr_base_and_unit_offset (dest, &offset); > > > if (dest_base == NULL_TREE) > > > - return false; > > > + return; > > > > > > /* Setup the worklist. */ > > > auto_vec<std::pair<tree, unsigned>> worklist; > > > @@ -1409,13 +1406,11 @@ optimize_aggr_zeroprop (gimple_stmt_iterator > > > *gsip, bool full_walk) > > > new_limit)); > > > } > > > > > > - if (optimize_aggr_zeroprop_1 (stmt, use_stmt, dest_base, offset, > > > - val, wi::to_poly_offset (len))) > > > - changed = true; > > > + optimize_aggr_zeroprop_1 (stmt, use_stmt, dest_base, offset, > > > + val, wi::to_poly_offset (len)); > > > } > > > } > > > > > > - return changed; > > > } > > > > > > /* Returns the pointer to the base of the object of the > > > @@ -1591,22 +1586,22 @@ same_for_assignment (tree src, tree dest) > > > /* Helper function for optimize_agr_copyprop. > > > For aggregate copies in USE_STMT, see if DEST > > > is on the lhs of USE_STMT and replace it with SRC. */ > > > -static bool > > > +static void > > > optimize_agr_copyprop_1 (gimple *stmt, gimple *use_stmt, > > > tree dest, tree src) > > > { > > > gcc_assert (gimple_assign_load_p (use_stmt) > > > && gimple_store_p (use_stmt)); > > > if (gimple_has_volatile_ops (use_stmt)) > > > - return false; > > > + return; > > > tree dest2 = gimple_assign_lhs (use_stmt); > > > tree src2 = gimple_assign_rhs1 (use_stmt); > > > /* If the new store is `src2 = src2;` skip over it. */ > > > if (same_for_assignment (src2, dest2)) > > > - return false; > > > + return; > > > src = new_src_based_on_copy (src2, dest, src); > > > if (!src) > > > - return false; > > > + return; > > > /* For 2 memory refences and using a temporary to do the copy, > > > don't remove the temporary as the 2 memory references might overlap. > > > Note t does not need to be decl as it could be field. > > > @@ -1634,7 +1629,7 @@ optimize_agr_copyprop_1 (gimple *stmt, gimple > > > *use_stmt, > > > tree len = TYPE_SIZE_UNIT (TREE_TYPE (src)); > > > if (len == NULL_TREE > > > || !tree_fits_poly_int64_p (len)) > > > - return false; > > > + return; > > > tree base1 = get_addr_base_and_unit_offset (dest2, &offset1); > > > tree base2 = get_addr_base_and_unit_offset (src, &offset2); > > > poly_int64 size = tree_to_poly_int64 (len); > > > @@ -1659,14 +1654,14 @@ optimize_agr_copyprop_1 (gimple *stmt, gimple > > > *use_stmt, > > > of overlapping. */ > > > if (maybe_lt (align1, size) > > > || maybe_lt (align2, size)) > > > - return false; > > > + return; > > > } > > > /* Make sure [offset1, offset1 + len - 1] does > > > not overlap with [offset2, offset2 + len - 1], > > > it is ok if they are at the same location though. */ > > > else if (ranges_maybe_overlap_p (offset1, size, offset2, size) > > > && !known_eq (offset2, offset1)) > > > - return false; > > > + return; > > > } > > > > > > if (dump_file && (dump_flags & TDF_DETAILS)) > > > @@ -1689,13 +1684,12 @@ optimize_agr_copyprop_1 (gimple *stmt, gimple > > > *use_stmt, > > > if (maybe_clean_or_replace_eh_stmt (orig_stmt, use_stmt)) > > > bitmap_set_bit (to_purge, gimple_bb (stmt)->index); > > > statistics_counter_event (cfun, "copy prop for aggregate", 1); > > > - return true; > > > } > > > > > > /* Helper function for optimize_agr_copyprop_1, propagate aggregates > > > into the arguments of USE_STMT if the argument matches with DEST; > > > replacing it with SRC. */ > > > -static bool > > > +static void > > > optimize_agr_copyprop_arg (gimple *defstmt, gcall *call, > > > tree dest, tree src) > > > { > > > @@ -1728,7 +1722,6 @@ optimize_agr_copyprop_arg (gimple *defstmt, gcall > > > *call, > > > } > > > if (changed) > > > update_stmt (call); > > > - return changed; > > > } > > > > > > /* Optimizes > > > @@ -1748,39 +1741,34 @@ optimize_agr_copyprop_arg (gimple *defstmt, gcall > > > *call, > > > call_func(..., SRC, ...); > > > > > > */ > > > -static bool > > > +static void > > > optimize_agr_copyprop (gimple_stmt_iterator *gsip) > > > { > > > gimple *stmt = gsi_stmt (*gsip); > > > if (gimple_has_volatile_ops (stmt)) > > > - return false; > > > + return; > > > > > > /* Can't prop if the statement could throw. */ > > > if (stmt_could_throw_p (cfun, stmt)) > > > - return false; > > > + return; > > > > > > tree dest = gimple_assign_lhs (stmt); > > > tree src = gimple_assign_rhs1 (stmt); > > > /* If the statement is `src = src;` then ignore it. */ > > > if (same_for_assignment (dest, src)) > > > - return false; > > > + return; > > > > > > tree vdef = gimple_vdef (stmt); > > > imm_use_iterator iter; > > > gimple *use_stmt; > > > - bool changed = false; > > > FOR_EACH_IMM_USE_STMT (use_stmt, iter, vdef) > > > { > > > if (gimple_assign_load_p (use_stmt) > > > - && gimple_store_p (use_stmt) > > > - && optimize_agr_copyprop_1 (stmt, use_stmt, dest, src)) > > > - changed = true; > > > - else if (is_gimple_call (use_stmt) > > > - && optimize_agr_copyprop_arg (stmt, as_a<gcall*>(use_stmt), > > > - dest, src)) > > > - changed = true; > > > + && gimple_store_p (use_stmt)) > > > + optimize_agr_copyprop_1 (stmt, use_stmt, dest, src); > > > + else if (is_gimple_call (use_stmt)) > > > + optimize_agr_copyprop_arg (stmt, as_a<gcall*>(use_stmt), dest, > > > src); > > > } > > > - return changed; > > > } > > > > > > /* Optimizes builtin memcmps for small constant sizes. > > > @@ -2180,8 +2168,7 @@ simplify_builtin_call (gimple_stmt_iterator *gsi_p, > > > tree callee2, bool full_walk > > > { > > > /* Try to prop the zeroing/value of the memset to memcpy > > > if the dest is an address and the value is a constant. */ > > > - if (optimize_aggr_zeroprop (gsi_p, full_walk)) > > > - return true; > > > + optimize_aggr_zeroprop (gsi_p, full_walk); > > > } > > > return simplify_builtin_memcpy_memset (gsi_p, as_a<gcall*>(stmt2)); > > > > > > @@ -5323,17 +5310,10 @@ pass_forwprop::execute (function *fun) > > > { > > > tree rhs1 = gimple_assign_rhs1 (stmt); > > > enum tree_code code = gimple_assign_rhs_code (stmt); > > > - if (gimple_store_p (stmt) && optimize_aggr_zeroprop > > > (&gsi, full_walk)) > > > - { > > > - changed = true; > > > - break; > > > - } > > > - if (gimple_assign_load_p (stmt) && gimple_store_p > > > (stmt) > > > - && optimize_agr_copyprop (&gsi)) > > > - { > > > - changed = true; > > > - break; > > > - } > > > + if (gimple_store_p (stmt)) > > > + optimize_aggr_zeroprop (&gsi, full_walk); > > > + if (gimple_assign_load_p (stmt) && gimple_store_p > > > (stmt)) > > > + optimize_agr_copyprop (&gsi); > > > > It looks to me that we can do > > > > if (gimple_store_p (stmt)) > > { > > optimize_aggr_zeroprop (&gsi, full_walk); > > // does this ever leave us with a non-store > > 'stmt'? need to do stmt = gsi_stmt (gsi)? > > Both optimize_aggr_zeroprop and optimize_agr_copyprop don't change > their gsi nor the statement that it points to. The argument being a > pointer was more of left over when they looked backwards. > I will post a separate patch to pass the stmt instead of gsi. > > Thanks, > Andrew > > > > if (gimple_assign_load_p (stmt)) > > optimize_agr_copyprop (&gsi); > > } > > else if (TREE_CODE_CLASS (code) == tcc_comparison) > > ... > > > > ? We only handle loads (ARRAY_REF) below.
Yes we only handle ARRAY_REF loads below but I think it should handle more, I filed PR 121997 for the missed optimization of simplify_count_zeroes where it is not an ARRAY_REF. Thanks, Andrew > > > > OK with such change. > > > > Richard. > > > > > if (TREE_CODE_CLASS (code) == tcc_comparison) > > > changed |= forward_propagate_into_comparison (&gsi); > > > -- > > > 2.43.0 > > >
