Message ID | ri6y2gqujvq.fsf@suse.cz |
---|---|
State | New |
Headers | show |
Series | ipa-sra: Do not remove return values needed because of non-call EH (PR 98690) | expand |
On Mon, Jan 18, 2021 at 8:27 PM Martin Jambor <mjambor@suse.cz> wrote: > > Hi, > > IPA-SRA already contains a check to figure out that an otherwise dead > parameter is actually required because of non-call exceptions, but it > is not present at the equivalent spot where SRA figures out whether > the return statement is used for anything useful. This patch adds > that condition there. > > Unfortunately, even though this patch should be good enough for any > normal (I'd even say reasonable) use of the compiler, it hints that > when the user manually switches all sorts of DCE, IPA-SRA would > probably leave behind problematic statements manipulating what > originally were return values, just like it does for parameters (PR > 93385). Fixing this properly might unfortunately be a separate issue > from the mentioned bug because the LHS of a call is changed during > call redirection and the caller often is not a clone. But I'll see > what I can do. > > Meanwhile, the patch below has been bootstrapped and tested on x86_64. > OK for trunk and then for the gcc-10 branch? OK. Thanks, Richard. > Thanks, > > Martin > > > gcc/ChangeLog: > > 2021-01-18 Martin Jambor <mjambor@suse.cz> > > PR ipa/98690 > * ipa-sra.c (ssa_name_only_returned_p): New parameter fun. Check > whether non-call exceptions allow removal of a statement. > (isra_analyze_call): Pass the appropriate function to > ssa_name_only_returned_p. > > gcc/testsuite/ChangeLog: > > 2021-01-18 Martin Jambor <mjambor@suse.cz> > > PR ipa/98690 > * g++.dg/ipa/pr98690.C: New test. > --- > gcc/ipa-sra.c | 20 +++++++++++--------- > gcc/testsuite/g++.dg/ipa/pr98690.C | 27 +++++++++++++++++++++++++++ > 2 files changed, 38 insertions(+), 9 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/ipa/pr98690.C > > diff --git a/gcc/ipa-sra.c b/gcc/ipa-sra.c > index 5d2c0dfce53..1571921cb48 100644 > --- a/gcc/ipa-sra.c > +++ b/gcc/ipa-sra.c > @@ -1952,13 +1952,13 @@ scan_function (cgraph_node *node, struct function *fun) > } > } > > -/* Return true if SSA_NAME NAME is only used in return statements, or if > - results of any operations it is involved in are only used in return > - statements. ANALYZED is a bitmap that tracks which SSA names we have > - already started investigating. */ > +/* Return true if SSA_NAME NAME of function described by FUN is only used in > + return statements, or if results of any operations it is involved in are > + only used in return statements. ANALYZED is a bitmap that tracks which SSA > + names we have already started investigating. */ > > static bool > -ssa_name_only_returned_p (tree name, bitmap analyzed) > +ssa_name_only_returned_p (function *fun, tree name, bitmap analyzed) > { > bool res = true; > imm_use_iterator imm_iter; > @@ -1978,8 +1978,9 @@ ssa_name_only_returned_p (tree name, bitmap analyzed) > break; > } > } > - else if ((is_gimple_assign (stmt) && !gimple_has_volatile_ops (stmt)) > - || gimple_code (stmt) == GIMPLE_PHI) > + else if (!stmt_unremovable_because_of_non_call_eh_p (fun, stmt) > + && ((is_gimple_assign (stmt) && !gimple_has_volatile_ops (stmt)) > + || gimple_code (stmt) == GIMPLE_PHI)) > { > /* TODO: And perhaps for const function calls too? */ > tree lhs; > @@ -1995,7 +1996,7 @@ ssa_name_only_returned_p (tree name, bitmap analyzed) > } > gcc_assert (!gimple_vdef (stmt)); > if (bitmap_set_bit (analyzed, SSA_NAME_VERSION (lhs)) > - && !ssa_name_only_returned_p (lhs, analyzed)) > + && !ssa_name_only_returned_p (fun, lhs, analyzed)) > { > res = false; > break; > @@ -2049,7 +2050,8 @@ isra_analyze_call (cgraph_edge *cs) > if (TREE_CODE (lhs) == SSA_NAME) > { > bitmap analyzed = BITMAP_ALLOC (NULL); > - if (ssa_name_only_returned_p (lhs, analyzed)) > + if (ssa_name_only_returned_p (DECL_STRUCT_FUNCTION (cs->caller->decl), > + lhs, analyzed)) > csum->m_return_returned = true; > BITMAP_FREE (analyzed); > } > diff --git a/gcc/testsuite/g++.dg/ipa/pr98690.C b/gcc/testsuite/g++.dg/ipa/pr98690.C > new file mode 100644 > index 00000000000..004418e5b40 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/ipa/pr98690.C > @@ -0,0 +1,27 @@ > +/* { dg-do run } */ > +/* { dg-options "-O2 -fnon-call-exceptions" } */ > + > +int g; > +volatile int v; > + > +static int * __attribute__((noinline)) > +almost_useless_return (void) > +{ > + v = 1; > + return &g; > +} > + > +static void __attribute__((noinline)) > +foo (void) > +{ > + int *p = almost_useless_return (); > + int i = *p; > + v = 2; > +} > + > +int > +main (int argc, char *argv[]) > +{ > + foo (); > + return 0; > +} > -- > 2.29.2 >
diff --git a/gcc/ipa-sra.c b/gcc/ipa-sra.c index 5d2c0dfce53..1571921cb48 100644 --- a/gcc/ipa-sra.c +++ b/gcc/ipa-sra.c @@ -1952,13 +1952,13 @@ scan_function (cgraph_node *node, struct function *fun) } } -/* Return true if SSA_NAME NAME is only used in return statements, or if - results of any operations it is involved in are only used in return - statements. ANALYZED is a bitmap that tracks which SSA names we have - already started investigating. */ +/* Return true if SSA_NAME NAME of function described by FUN is only used in + return statements, or if results of any operations it is involved in are + only used in return statements. ANALYZED is a bitmap that tracks which SSA + names we have already started investigating. */ static bool -ssa_name_only_returned_p (tree name, bitmap analyzed) +ssa_name_only_returned_p (function *fun, tree name, bitmap analyzed) { bool res = true; imm_use_iterator imm_iter; @@ -1978,8 +1978,9 @@ ssa_name_only_returned_p (tree name, bitmap analyzed) break; } } - else if ((is_gimple_assign (stmt) && !gimple_has_volatile_ops (stmt)) - || gimple_code (stmt) == GIMPLE_PHI) + else if (!stmt_unremovable_because_of_non_call_eh_p (fun, stmt) + && ((is_gimple_assign (stmt) && !gimple_has_volatile_ops (stmt)) + || gimple_code (stmt) == GIMPLE_PHI)) { /* TODO: And perhaps for const function calls too? */ tree lhs; @@ -1995,7 +1996,7 @@ ssa_name_only_returned_p (tree name, bitmap analyzed) } gcc_assert (!gimple_vdef (stmt)); if (bitmap_set_bit (analyzed, SSA_NAME_VERSION (lhs)) - && !ssa_name_only_returned_p (lhs, analyzed)) + && !ssa_name_only_returned_p (fun, lhs, analyzed)) { res = false; break; @@ -2049,7 +2050,8 @@ isra_analyze_call (cgraph_edge *cs) if (TREE_CODE (lhs) == SSA_NAME) { bitmap analyzed = BITMAP_ALLOC (NULL); - if (ssa_name_only_returned_p (lhs, analyzed)) + if (ssa_name_only_returned_p (DECL_STRUCT_FUNCTION (cs->caller->decl), + lhs, analyzed)) csum->m_return_returned = true; BITMAP_FREE (analyzed); } diff --git a/gcc/testsuite/g++.dg/ipa/pr98690.C b/gcc/testsuite/g++.dg/ipa/pr98690.C new file mode 100644 index 00000000000..004418e5b40 --- /dev/null +++ b/gcc/testsuite/g++.dg/ipa/pr98690.C @@ -0,0 +1,27 @@ +/* { dg-do run } */ +/* { dg-options "-O2 -fnon-call-exceptions" } */ + +int g; +volatile int v; + +static int * __attribute__((noinline)) +almost_useless_return (void) +{ + v = 1; + return &g; +} + +static void __attribute__((noinline)) +foo (void) +{ + int *p = almost_useless_return (); + int i = *p; + v = 2; +} + +int +main (int argc, char *argv[]) +{ + foo (); + return 0; +}