On Wed, Sep 24, 2025 at 5:33 AM Richard Biener <[email protected]> wrote: > > On Mon, Sep 22, 2025 at 5:36 PM Andrew Pinski > <[email protected]> wrote: > > > > This code dates before gimple tuples was around. So it uses both > > MODIFY_EXPR and > > INDIRECT_REF :). > > For `__builtin_va_start(ptr, 0)` it exands into: > > ``` > > _t = __builtin_next_arg (0); > > *ptr = _t; > > ``` > > We need to get a new VDEF for the next arg call so we don't need to do a > > ssa update too. > > > > For `__builtin_va_copy(ptr, b)`, it expands into: > > ``` > > *ptr = b; > > ``` > > Which is still a store. > > > > For `__builtin_va_end(ptr)`, we change it into a GIMPLE_NOP. > > > > Since we don't return a value, we need to also set TODO_update_address_taken > > on the todo manually too. > > We do have to? That's an optimization. > > Looks OK to me, but I'm confused about the above.
Let me explain it differently because I was not clear. Before optimize_stdarg_builtin would return a tree and then pass_fold_builtins::execute would set todo to include TODO_update_address_taken for a non-null result. Since now optimize_stdarg_builtin returns a bool, the path that had set the todo is no longer taken (the result variable is still NULL). Setting the todo to include TODO_update_address_taken when the call to optimize_stdarg_builtin succeeds to be able to get the same effect as before. That is: before we had: ``` result = optimize_stdarg_builtin(...); ... if (!result) continue; todo |= TODO_update_address_taken; ``` Now after this change we have: result = NULL_TREE; ... if (optimize_stdarg_builtin(...)) todo |= TODO_update_address_taken; if (!result) continue; ``` Also since optimize_stdarg_builtin will remove the part of taking an address of a local variable and in this case fab is the last pass where this happens, TODO_update_address_taken is needed to change the va args in some cases to not forced to memory variables. Thanks, Andrew Pinski > > Richard. > > > > > Note this code will be moved into gimple_fold later on. This is part of the > > reason for updating this code. The other side is this simplifies the code > > too. > > > > gcc/ChangeLog: > > > > * tree-ssa-ccp.cc (optimize_stdarg_builtin): Mannually create the > > gimple statements instead of depending on the gimplifier. > > (pass_fold_builtins::execute): Handle updated call to > > optimize_stdarg_builtin. > > > > Signed-off-by: Andrew Pinski <[email protected]> > > --- > > gcc/tree-ssa-ccp.cc | 87 ++++++++++++++++++++++++++++++++++----------- > > 1 file changed, 66 insertions(+), 21 deletions(-) > > > > diff --git a/gcc/tree-ssa-ccp.cc b/gcc/tree-ssa-ccp.cc > > index 1fbc3de2bfe..c9ffd2af85c 100644 > > --- a/gcc/tree-ssa-ccp.cc > > +++ b/gcc/tree-ssa-ccp.cc > > @@ -3175,14 +3175,16 @@ optimize_stack_restore (gimple_stmt_iterator i) > > /* If va_list type is a simple pointer and nothing special is needed, > > optimize __builtin_va_start (&ap, 0) into ap = __builtin_next_arg (0), > > __builtin_va_end (&ap) out as NOP and __builtin_va_copy into a simple > > - pointer assignment. */ > > + pointer assignment. Returns true if a change happened. */ > > > > -static tree > > -optimize_stdarg_builtin (gimple *call) > > +static bool > > +optimize_stdarg_builtin (gimple_stmt_iterator *gsi, gimple *call) > > { > > tree callee, lhs, rhs, cfun_va_list; > > bool va_list_simple_ptr; > > location_t loc = gimple_location (call); > > + gimple *nstmt0, *nstmt; > > + tree tlhs, oldvdef, newvdef; > > > > callee = gimple_call_fndecl (call); > > > > @@ -3197,48 +3199,90 @@ optimize_stdarg_builtin (gimple *call) > > if (!va_list_simple_ptr > > || targetm.expand_builtin_va_start != NULL > > || !builtin_decl_explicit_p (BUILT_IN_NEXT_ARG)) > > - return NULL_TREE; > > + return false; > > > > if (gimple_call_num_args (call) != 2) > > - return NULL_TREE; > > + return false; > > > > lhs = gimple_call_arg (call, 0); > > if (!POINTER_TYPE_P (TREE_TYPE (lhs)) > > || TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (lhs))) > > != TYPE_MAIN_VARIANT (cfun_va_list)) > > - return NULL_TREE; > > + return false; > > + /* Create `tlhs = __builtin_next_arg(0);`. */ > > + tlhs = make_ssa_name (cfun_va_list); > > + nstmt0 = gimple_build_call (builtin_decl_explicit > > (BUILT_IN_NEXT_ARG), 1, integer_zero_node); > > + lhs = fold_build2 (MEM_REF, cfun_va_list, lhs, build_zero_cst > > (TREE_TYPE (lhs))); > > + gimple_call_set_lhs (nstmt0, tlhs); > > + gimple_set_location (nstmt0, loc); > > + gimple_move_vops (nstmt0, call); > > + gsi_replace (gsi, nstmt0, false); > > + oldvdef = gimple_vdef (nstmt0); > > + newvdef = make_ssa_name (gimple_vop (cfun), nstmt0); > > + gimple_set_vdef (nstmt0, newvdef); > > + > > + /* Create `*lhs = tlhs;`. */ > > + nstmt = gimple_build_assign (lhs, tlhs); > > + gimple_set_location (nstmt, loc); > > + gimple_set_vuse (nstmt, newvdef); > > + gimple_set_vdef (nstmt, oldvdef); > > + SSA_NAME_DEF_STMT (oldvdef) = nstmt; > > + gsi_insert_after (gsi, nstmt, GSI_NEW_STMT); > > > > - lhs = build_fold_indirect_ref_loc (loc, lhs); > > - rhs = build_call_expr_loc (loc, builtin_decl_explicit > > (BUILT_IN_NEXT_ARG), > > - 1, integer_zero_node); > > - rhs = fold_convert_loc (loc, TREE_TYPE (lhs), rhs); > > - return build2 (MODIFY_EXPR, TREE_TYPE (lhs), lhs, rhs); > > + if (dump_file && (dump_flags & TDF_DETAILS)) > > + { > > + fprintf (dump_file, "Simplified\n "); > > + print_gimple_stmt (dump_file, call, 0, dump_flags); > > + fprintf (dump_file, "into\n "); > > + print_gimple_stmt (dump_file, nstmt0, 0, dump_flags); > > + fprintf (dump_file, " "); > > + print_gimple_stmt (dump_file, nstmt, 0, dump_flags); > > + } > > + return true; > > > > case BUILT_IN_VA_COPY: > > if (!va_list_simple_ptr) > > - return NULL_TREE; > > + return false; > > > > if (gimple_call_num_args (call) != 2) > > - return NULL_TREE; > > + return false; > > > > lhs = gimple_call_arg (call, 0); > > if (!POINTER_TYPE_P (TREE_TYPE (lhs)) > > || TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (lhs))) > > != TYPE_MAIN_VARIANT (cfun_va_list)) > > - return NULL_TREE; > > - > > - lhs = build_fold_indirect_ref_loc (loc, lhs); > > + return false; > > rhs = gimple_call_arg (call, 1); > > if (TYPE_MAIN_VARIANT (TREE_TYPE (rhs)) > > != TYPE_MAIN_VARIANT (cfun_va_list)) > > - return NULL_TREE; > > + return false; > > > > - rhs = fold_convert_loc (loc, TREE_TYPE (lhs), rhs); > > - return build2 (MODIFY_EXPR, TREE_TYPE (lhs), lhs, rhs); > > + lhs = fold_build2 (MEM_REF, cfun_va_list, lhs, build_zero_cst > > (TREE_TYPE (lhs))); > > + nstmt = gimple_build_assign (lhs, rhs); > > + gimple_set_location (nstmt, loc); > > + gimple_move_vops (nstmt, call); > > + gsi_replace (gsi, nstmt, false); > > + > > + if (dump_file && (dump_flags & TDF_DETAILS)) > > + { > > + fprintf (dump_file, "Simplified\n "); > > + print_gimple_stmt (dump_file, call, 0, dump_flags); > > + fprintf (dump_file, "into\n "); > > + print_gimple_stmt (dump_file, nstmt, 0, dump_flags); > > + } > > + return true; > > > > case BUILT_IN_VA_END: > > /* No effect, so the statement will be deleted. */ > > - return integer_zero_node; > > + if (dump_file && (dump_flags & TDF_DETAILS)) > > + { > > + fprintf (dump_file, "Removed\n "); > > + print_gimple_stmt (dump_file, call, 0, dump_flags); > > + } > > + unlink_stmt_vdef (call); > > + release_defs (call); > > + gsi_replace (gsi, gimple_build_nop (), true); > > + return true; > > > > default: > > gcc_unreachable (); > > @@ -4426,7 +4470,8 @@ pass_fold_builtins::execute (function *fun) > > case BUILT_IN_VA_END: > > case BUILT_IN_VA_COPY: > > /* These shouldn't be folded before pass_stdarg. */ > > - result = optimize_stdarg_builtin (stmt); > > + if (optimize_stdarg_builtin (&i, stmt)) > > + todoflags |= TODO_update_address_taken; > > break; > > > > default:; > > -- > > 2.43.0 > >
