On Mon, Oct 13, 2025 at 11:44 PM Richard Biener <[email protected]> wrote: > > On Tue, Oct 14, 2025 at 8:38 AM Andrew Pinski > <[email protected]> wrote: > > > > On Mon, Oct 13, 2025 at 11:28 PM Richard Biener > > <[email protected]> wrote: > > > > > > On Tue, Oct 14, 2025 at 5:35 AM Andrew Pinski > > > <[email protected]> wrote: > > > > > > > > When DCE removes the lhs of __builtin_stack_save, we can also > > > > remove the function itself. This adds that to gimple fold. > > > > > > > > Bootstrapped and tested on x86_64-linux-gnu. > > > > > > Huh, this makes me wonder why it's so difficult to remove the whole stmt > > > in DCE > > > rather than just the LHS? DCE-by-replacing-with-nop is a bit ugly. > > > > The problem is in fold_stmt, you can't remove the current statement > > because all callers think fold_stmt will stay the current statement. > > Changing that is much harder. > > This pattern of replacing with gimple_build_nop is used all over > > gimple-fold.cc already. > > See gimple_fold_builtin_memory_op, replace_call_with_value and > > gimplify_and_update_call_from_tree for examples of where this has been > > there before. > > Yes, I mean you say DCE removes the LHS of the call, why can't it remove > the call at that point?
Oh I didn't think there were special cases there. I now see there are a few; IFN_GOMP_SIMD_LANE/IFN_ASAN_POISON are there. Will submit a new patch tomorrow then. Thanks, Andrew > > > Thanks, > > Andrew Pinski > > > > > > > > > > > PR tree-optimization/122037 > > > > > > > > gcc/ChangeLog: > > > > > > > > * gimple-fold.cc (gimple_fold_builtin_stack_save): New function. > > > > (gimple_fold_builtin): Call gimple_fold_builtin_stack_save > > > > for __builtin_stack_save. > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > * gcc.dg/tree-ssa/vla-1.c: New test. > > > > > > > > Signed-off-by: Andrew Pinski <[email protected]> > > > > --- > > > > gcc/gimple-fold.cc | 21 +++++++++++++++++++++ > > > > gcc/testsuite/gcc.dg/tree-ssa/vla-1.c | 14 ++++++++++++++ > > > > 2 files changed, 35 insertions(+) > > > > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/vla-1.c > > > > > > > > diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc > > > > index edcc04adc08..5f88cce7760 100644 > > > > --- a/gcc/gimple-fold.cc > > > > +++ b/gcc/gimple-fold.cc > > > > @@ -5372,6 +5372,25 @@ gimple_fold_builtin_stdarg (gimple_stmt_iterator > > > > *gsi, gcall *call) > > > > } > > > > } > > > > > > > > +/* Fold __builtin_stack_save if possible. Returns true if it was > > > > folded. > > > > + Currently this removes the call if the lhs is unused. */ > > > > +static bool > > > > +gimple_fold_builtin_stack_save (gimple_stmt_iterator *gsi, gcall *call) > > > > +{ > > > > + if (gimple_call_num_args (call) != 0) > > > > + return false; > > > > + > > > > + if (gimple_call_lhs (call)) > > > > + return false; > > > > + > > > > + // Remove the call since the result is unused. > > > > + unlink_stmt_vdef (call); > > > > + release_defs (call); > > > > + gsi_replace (gsi, gimple_build_nop (), true); > > > > + > > > > + return true; > > > > +} > > > > + > > > > /* Fold the non-target builtin at *GSI and return whether any > > > > simplification > > > > was made. */ > > > > > > > > @@ -5390,6 +5409,8 @@ gimple_fold_builtin (gimple_stmt_iterator *gsi) > > > > enum built_in_function fcode = DECL_FUNCTION_CODE (callee); > > > > switch (fcode) > > > > { > > > > + case BUILT_IN_STACK_SAVE: > > > > + return gimple_fold_builtin_stack_save (gsi, stmt); > > > > case BUILT_IN_VA_START: > > > > case BUILT_IN_VA_END: > > > > case BUILT_IN_VA_COPY: > > > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vla-1.c > > > > b/gcc/testsuite/gcc.dg/tree-ssa/vla-1.c > > > > new file mode 100644 > > > > index 00000000000..ec19babebc4 > > > > --- /dev/null > > > > +++ b/gcc/testsuite/gcc.dg/tree-ssa/vla-1.c > > > > @@ -0,0 +1,14 @@ > > > > +/* { dg-do compile } */ > > > > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > > > > +/* PR tree-optimization/122037 */ > > > > + > > > > +void bar1 (char *, int) __attribute__((noreturn)); > > > > +void foo1 (int size) > > > > +{ > > > > + char temp[size]; > > > > + temp[size-1] = '\0'; > > > > + bar1 (temp, size); > > > > +} > > > > + > > > > +/* The call to __builtin_stack_save should have been removed. */ > > > > +/* { dg-final { scan-tree-dump-not "__builtin_stack_save " "optimized" > > > > } } */ > > > > -- > > > > 2.43.0 > > > >
