On Fri, Sep 6, 2024 at 3:00 AM Andrew Pinski <pins...@gmail.com> wrote:
>
> On Thu, Sep 5, 2024 at 12:26 AM Richard Biener
> <richard.guent...@gmail.com> wrote:
> >
> > On Thu, Sep 5, 2024 at 8:25 AM Andrew Pinski <quic_apin...@quicinc.com>
> > wrote:
> > >
> > > When optimize_memcpy was added in r7-5443-g7b45d0dfeb5f85,
> > > a path was added such that a statement was turned into a non-throwing
> > > statement and maybe_clean_or_replace_eh_stmt/gimple_purge_dead_eh_edges
> > > would not be called for that statement.
> > > This adds these calls to that path.
> > >
> > > Bootstrapped and tested on x86_64-linux-gnu with no regressions.
> > >
> > > Ok? For the trunk, 14, 13 and 12 branches?
> >
> > I wonder if this can be somehow integrated better with the existing
> >
> > old_stmt = stmt;
> > stmt = gsi_stmt (i);
> > update_stmt (stmt);
> >
> > if (maybe_clean_or_replace_eh_stmt (old_stmt, stmt)
> > && gimple_purge_dead_eh_edges (bb))
> > cfg_changed = true;
> >
> > which frankly looks odd - update_stmt shouldn't ever change stmt. Maybe
> > moving the old_stmt assign before the switch works?
>
> I agree it looks odd/wrong. But only moving the assignment before the
> switch does not fix this issue since if we don't have a builtin (which
> we have in this case, it is a memcpy like statement):
> __trans_tmp_2 = MEM[(const struct RefitOption &)__val_5(D)];
>
> I have a set of patches to refactor this code to simplify and fix the
> issue with the update_stmt and more (since there are issues with the
> atomic replacements too).
If it gets too big for branches consider the original change approved for
backporting (you might want to start with pushing that to trunk and then
refactoring). Looking I think the memcpy handling is simply misplaced
in the switch as it does more than all of the rest.
> >
> > > PR tree-optimization/116601
> > >
> > > gcc/ChangeLog:
> > >
> > > * tree-ssa-ccp.cc (pass_fold_builtins::execute): Cleanup eh
> > > after optimize_memcpy on a mem statement.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > * g++.dg/torture/except-2.C: New test.
> > >
> > > Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> > > ---
> > > gcc/testsuite/g++.dg/torture/except-2.C | 18 ++++++++++++++++++
> > > gcc/tree-ssa-ccp.cc | 11 +++++++++--
> > > 2 files changed, 27 insertions(+), 2 deletions(-)
> > > create mode 100644 gcc/testsuite/g++.dg/torture/except-2.C
> > >
> > > diff --git a/gcc/testsuite/g++.dg/torture/except-2.C
> > > b/gcc/testsuite/g++.dg/torture/except-2.C
> > > new file mode 100644
> > > index 00000000000..d896937a118
> > > --- /dev/null
> > > +++ b/gcc/testsuite/g++.dg/torture/except-2.C
> > > @@ -0,0 +1,18 @@
> > > +// { dg-do compile }
> > > +// { dg-additional-options "-fexceptions -fnon-call-exceptions" }
> > > +// PR tree-optimization/116601
> > > +
> > > +struct RefitOption {
> > > + char subtype;
> > > + int string;
> > > +} n;
> > > +void h(RefitOption);
> > > +void k(RefitOption *__val)
> > > +{
> > > + try {
> > > + *__val = RefitOption{};
> > > + RefitOption __trans_tmp_2 = *__val;
> > > + h(__trans_tmp_2);
> > > + }
> > > + catch(...){}
> > > +}
> > > diff --git a/gcc/tree-ssa-ccp.cc b/gcc/tree-ssa-ccp.cc
> > > index 44711018e0e..3cd385f476b 100644
> > > --- a/gcc/tree-ssa-ccp.cc
> > > +++ b/gcc/tree-ssa-ccp.cc
> > > @@ -4325,8 +4325,15 @@ pass_fold_builtins::execute (function *fun)
> > > if (gimple_code (stmt) != GIMPLE_CALL)
> > > {
> > > if (gimple_assign_load_p (stmt) && gimple_store_p (stmt))
> > > - optimize_memcpy (&i, gimple_assign_lhs (stmt),
> > > - gimple_assign_rhs1 (stmt), NULL_TREE);
> > > + {
> > > + optimize_memcpy (&i, gimple_assign_lhs (stmt),
> > > + gimple_assign_rhs1 (stmt), NULL_TREE);
> > > + old_stmt = stmt;
> > > + stmt = gsi_stmt (i);
> > > + if (maybe_clean_or_replace_eh_stmt (old_stmt, stmt)
> > > + && gimple_purge_dead_eh_edges (bb))
> > > + cfg_changed = true;
> > > + }
> > > gsi_next (&i);
> > > continue;
> > > }
> > > --
> > > 2.43.0
> > >
