On Thu, Oct 2, 2025 at 1:46 AM Richard Biener <[email protected]> wrote: > > On Wed, Sep 24, 2025 at 11:11 AM Andrew Pinski > <[email protected]> wrote: > > > > This moves the va_args functions folding to gimple-fold. Right now this is > > still > > kept for the last folding but later it can move sometime after stdargs pass > > is run by > > checking PROP_gimple_lva instead. > > Also for forwprop, if a folding happens for the last folding we need to > > maybe update the > > variables as non-addressable like what is done in fab. It was originally > > added in fab for > > __builtin_sincos->__builtin_cexpi folding (PR39643). After the removal of > > fab, this will > > also be the last pass which updates address taken too. > > > > gcc/ChangeLog: > > > > * gimple-fold.cc (gimple_fold_builtin_stdarg): New function, > > moved from tree-ssa-ccp.cc (optimize_stdarg_builtin). > > (gimple_fold_builtin): Call gimple_fold_builtin_stdarg for > > va_start, va_copy and va_end. > > * tree-ssa-ccp.cc (optimize_stdarg_builtin): Remove. > > (pass_fold_builtins::execute): Remove handling of > > va_start, va_copy and va_end. > > * tree-ssa-forwprop.cc (pass_forwprop::execute): Update > > todos if fold_stmt return true if last forwprop to include > > TODO_update_address_taken. > > > > Signed-off-by: Andrew Pinski <[email protected]> > > --- > > gcc/gimple-fold.cc | 125 ++++++++++++++++++++++++++++++++++++++ > > gcc/tree-ssa-ccp.cc | 126 --------------------------------------- > > gcc/tree-ssa-forwprop.cc | 5 ++ > > 3 files changed, 130 insertions(+), 126 deletions(-) > > > > diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc > > index a12fb5e8eab..2f64de2fb41 100644 > > --- a/gcc/gimple-fold.cc > > +++ b/gcc/gimple-fold.cc > > @@ -5252,6 +5252,127 @@ gimple_fold_builtin_assume_aligned > > (gimple_stmt_iterator *gsi) > > return true; > > } > > > > +/* 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. Returns true if a change happened. */ > > + > > +static bool > > +gimple_fold_builtin_stdarg (gimple_stmt_iterator *gsi, gcall *call) > > +{ > > + /* These shouldn't be folded before pass_stdarg. */ > > + if (!(cfun->curr_properties & PROP_last_full_fold)) > > + return false; > > + > > + 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); > > + > > + cfun_va_list = targetm.fn_abi_va_list (callee); > > + va_list_simple_ptr = POINTER_TYPE_P (cfun_va_list) > > + && (TREE_TYPE (cfun_va_list) == void_type_node > > + || TREE_TYPE (cfun_va_list) == char_type_node); > > + > > + switch (DECL_FUNCTION_CODE (callee)) > > + { > > + case BUILT_IN_VA_START: > > + if (!va_list_simple_ptr > > + || targetm.expand_builtin_va_start != NULL > > + || !builtin_decl_explicit_p (BUILT_IN_NEXT_ARG)) > > + return false; > > + > > + if (gimple_call_num_args (call) != 2) > > + 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 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); > > + > > + 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 false; > > + > > + if (gimple_call_num_args (call) != 2) > > + 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 false; > > + rhs = gimple_call_arg (call, 1); > > + if (TYPE_MAIN_VARIANT (TREE_TYPE (rhs)) > > + != TYPE_MAIN_VARIANT (cfun_va_list)) > > + return false; > > + > > + 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. */ > > + 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 (); > > + } > > +} > > + > > /* Fold the non-target builtin at *GSI and return whether any > > simplification > > was made. */ > > > > @@ -5270,6 +5391,10 @@ gimple_fold_builtin (gimple_stmt_iterator *gsi) > > enum built_in_function fcode = DECL_FUNCTION_CODE (callee); > > switch (fcode) > > { > > + case BUILT_IN_VA_START: > > + case BUILT_IN_VA_END: > > + case BUILT_IN_VA_COPY: > > + return gimple_fold_builtin_stdarg (gsi, stmt); > > case BUILT_IN_BCMP: > > return gimple_fold_builtin_bcmp (gsi); > > case BUILT_IN_BCOPY: > > diff --git a/gcc/tree-ssa-ccp.cc b/gcc/tree-ssa-ccp.cc > > index b69a8b64a61..2d16395ac68 100644 > > --- a/gcc/tree-ssa-ccp.cc > > +++ b/gcc/tree-ssa-ccp.cc > > @@ -3085,124 +3085,6 @@ make_pass_ccp (gcc::context *ctxt) > > return new pass_ccp (ctxt); > > } > > > > -/* 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. Returns true if a change happened. */ > > - > > -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); > > - > > - cfun_va_list = targetm.fn_abi_va_list (callee); > > - va_list_simple_ptr = POINTER_TYPE_P (cfun_va_list) > > - && (TREE_TYPE (cfun_va_list) == void_type_node > > - || TREE_TYPE (cfun_va_list) == char_type_node); > > - > > - switch (DECL_FUNCTION_CODE (callee)) > > - { > > - case BUILT_IN_VA_START: > > - if (!va_list_simple_ptr > > - || targetm.expand_builtin_va_start != NULL > > - || !builtin_decl_explicit_p (BUILT_IN_NEXT_ARG)) > > - return false; > > - > > - if (gimple_call_num_args (call) != 2) > > - 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 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); > > - > > - 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 false; > > - > > - if (gimple_call_num_args (call) != 2) > > - 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 false; > > - rhs = gimple_call_arg (call, 1); > > - if (TYPE_MAIN_VARIANT (TREE_TYPE (rhs)) > > - != TYPE_MAIN_VARIANT (cfun_va_list)) > > - return false; > > - > > - 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. */ > > - 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 (); > > - } > > -} > > - > > - > > /* Convert > > _1 = __atomic_fetch_or_* (ptr_6, 1, _3); > > _7 = ~_1; > > @@ -4339,14 +4221,6 @@ pass_fold_builtins::execute (function *fun) > > false); > > break; > > > > - case BUILT_IN_VA_START: > > - case BUILT_IN_VA_END: > > - case BUILT_IN_VA_COPY: > > - /* These shouldn't be folded before pass_stdarg. */ > > - if (optimize_stdarg_builtin (&i, stmt)) > > - todoflags |= TODO_update_address_taken; > > - break; > > - > > default:; > > } > > > > diff --git a/gcc/tree-ssa-forwprop.cc b/gcc/tree-ssa-forwprop.cc > > index c7ac96a28aa..f72aac21980 100644 > > --- a/gcc/tree-ssa-forwprop.cc > > +++ b/gcc/tree-ssa-forwprop.cc > > @@ -5464,6 +5464,11 @@ pass_forwprop::execute (function *fun) > > if (fold_stmt (&gsi, fwprop_ssa_val, simple_dce_worklist)) > > { > > changed = true; > > + /* There is no updating of the address > > + taken after the last forwprop so update > > + the addresses when a folding happened. */ > > + if (last_p) > > + todoflags |= TODO_update_address_taken; > > The patch is OK. As a followup we might want to consider whether there's > a cheap enough way to see in which cases address-takens are elided. > memcpy() -> assignment transforms come to my mind here, but I > cannot think of anything that wasn't a call before - so maybe it's > enough to add a && was_call (for was_noreturn we already compute > was_call).?
I filed PR 122143 to remind myself to do this followup. yes also was_call might be a good first check but I wonder if we can do slightly better and still be cheap. I will submit the was_call part at least and then think about if there is some extra needed. Thanks, Andrew > > > stmt = gsi_stmt (gsi); > > /* Cleanup the CFG if we simplified a condition to > > true or false. */ > > -- > > 2.43.0 > >
