On Mon, Jul 31, 2023 at 7:05 PM Martin Jambor <mjam...@suse.cz> wrote:
>
> Hi,
>
> when IPA-SRA detects whether a parameter passed by reference is
> written to, it does not special case CLOBBERs which means it often
> bails out unnecessarily, especially when dealing with C++ destructors.
> Fixed by the obvious continue in the two relevant loops.
>
> The (slightly) more complex testcases in the PR need surprisingly more
> effort but the simple one can be fixed now easily by this patch and I'll
> work on the others incrementally.
>
> Bootstrapped and currently undergoing testsuite run on x86_64-linux. OK
> if it passes too?
LGTM, btw - how are the clobbers handled during transform?
> Thanks,
>
> Martin
>
>
>
>
> gcc/ChangeLog:
>
> 2023-07-31 Martin Jambor <mjam...@suse.cz>
>
> PR ipa/110378
> * ipa-sra.cc (isra_track_scalar_value_uses): Ignore clobbers.
> (ptr_parm_has_nonarg_uses): Likewise.
>
> gcc/testsuite/ChangeLog:
>
> 2023-07-31 Martin Jambor <mjam...@suse.cz>
>
> PR ipa/110378
> * g++.dg/ipa/pr110378-1.C: New test.
> ---
> gcc/ipa-sra.cc | 6 ++--
> gcc/testsuite/g++.dg/ipa/pr110378-1.C | 47 +++++++++++++++++++++++++++
> 2 files changed, 51 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/ipa/pr110378-1.C
>
> diff --git a/gcc/ipa-sra.cc b/gcc/ipa-sra.cc
> index c35e03b7abd..edba364f56e 100644
> --- a/gcc/ipa-sra.cc
> +++ b/gcc/ipa-sra.cc
> @@ -898,7 +898,8 @@ isra_track_scalar_value_uses (function *fun, cgraph_node
> *node, tree name,
>
> FOR_EACH_IMM_USE_STMT (stmt, imm_iter, name)
> {
> - if (is_gimple_debug (stmt))
> + if (is_gimple_debug (stmt)
> + || gimple_clobber_p (stmt))
> continue;
>
> /* TODO: We could handle at least const builtin functions like
> arithmetic
> @@ -1056,7 +1057,8 @@ ptr_parm_has_nonarg_uses (cgraph_node *node, function
> *fun, tree parm,
> unsigned uses_ok = 0;
> use_operand_p use_p;
>
> - if (is_gimple_debug (stmt))
> + if (is_gimple_debug (stmt)
> + || gimple_clobber_p (stmt))
> continue;
>
> if (gimple_assign_single_p (stmt))
> diff --git a/gcc/testsuite/g++.dg/ipa/pr110378-1.C
> b/gcc/testsuite/g++.dg/ipa/pr110378-1.C
> new file mode 100644
> index 00000000000..aabe326b8b2
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/ipa/pr110378-1.C
> @@ -0,0 +1,47 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-ipa-sra -fdump-tree-optimized-slim" } */
> +
> +/* Test that even though destructors end with clobbering all of *this, it
> + should not prevent IPA-SRA. */
> +
> +namespace {
> +
> + class foo
> + {
> + public:
> + int *a;
> + foo(int c)
> + {
> + a = new int[c];
> + a[0] = 4;
> + }
> + __attribute__((noinline)) ~foo();
> + int f ()
> + {
> + return a[0] + 1;
> + }
> + };
> +
> + volatile int v1 = 4;
> +
> + __attribute__((noinline)) foo::~foo()
> + {
> + delete[] a;
> + return;
> + }
> +
> +
> +}
> +
> +volatile int v2 = 20;
> +
> +int test (void)
> +{
> + foo shouldnotexist(v2);
> + v2 = shouldnotexist.f();
> + return 0;
> +}
> +
> +
> +/* { dg-final { scan-ipa-dump "Will split parameter 0" "sra" } } */
> +/* { dg-final { scan-tree-dump-not "shouldnotexist" "optimized" } } */
> --
> 2.41.0
>
