On Fri, Jul 12, 2024 at 7:24 AM liuhongt <hongtao....@intel.com> wrote:
>
> >- _5 = __atomic_fetch_or_8 (&set_work_pending_p, 1, 0);
> >- # DEBUG old => (long int) _5
> >+ _6 = .ATOMIC_BIT_TEST_AND_SET (&set_work_pending_p, 0, 1, 0,
> >__atomic_fetch_or_8);
> >+ # DEBUG old => NULL
> > # DEBUG BEGIN_STMT
> >- # DEBUG D#2 => _5 & 1
> >+ # DEBUG D#2 => NULL
> >...
> >- _10 = ~_5;
> >- _8 = (_Bool) _10;
> >- # DEBUG ret => _8
> >+ _8 = _6 == 0;
> >+ # DEBUG ret => (_Bool) _10
> >
> >confirmed. convert_atomic_bit_not does this, it checks for single_use
> >and removes the def, failing to release the name (which would fix this up
> >IIRC).
> >
> >Note the function removes stmts in "wrong" order (before uses of LHS
> >are removed), so it requires larger surgery. And it leaks SSA names.
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
> Ok for trunk?
OK.
Richard.
> gcc/ChangeLog:
>
> PR target/115872
> * tree-ssa-ccp.cc (convert_atomic_bit_not): Remove use_stmt after
> use_nop_stmt is removed.
> (optimize_atomic_bit_test_and): Ditto.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/i386/pr115872.c: New test.
> ---
> gcc/testsuite/gcc.target/i386/pr115872.c | 16 ++++++++++++++++
> gcc/tree-ssa-ccp.cc | 12 ++++++++----
> 2 files changed, 24 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/i386/pr115872.c
>
> diff --git a/gcc/testsuite/gcc.target/i386/pr115872.c
> b/gcc/testsuite/gcc.target/i386/pr115872.c
> new file mode 100644
> index 00000000000..937004456d3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr115872.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -g" } */
> +
> +long set_work_pending_p;
> +_Bool set_work_pending() {
> + _Bool __trans_tmp_1;
> + long mask = 1, old = __atomic_fetch_or(&set_work_pending_p, mask, 0);
> + __trans_tmp_1 = old & mask;
> + return !__trans_tmp_1;
> +}
> +void __queue_work() {
> + _Bool ret = set_work_pending();
> + if (ret)
> + __queue_work();
> +}
> +
> diff --git a/gcc/tree-ssa-ccp.cc b/gcc/tree-ssa-ccp.cc
> index 3749126b5f7..de83d26d311 100644
> --- a/gcc/tree-ssa-ccp.cc
> +++ b/gcc/tree-ssa-ccp.cc
> @@ -3332,9 +3332,10 @@ convert_atomic_bit_not (enum internal_fn fn, gimple
> *use_stmt,
> return nullptr;
>
> gimple_stmt_iterator gsi;
> - gsi = gsi_for_stmt (use_stmt);
> - gsi_remove (&gsi, true);
> tree var = make_ssa_name (TREE_TYPE (lhs));
> + /* use_stmt need to be removed after use_nop_stmt,
> + so use_lhs can be released. */
> + gimple *use_stmt_removal = use_stmt;
> use_stmt = gimple_build_assign (var, BIT_AND_EXPR, lhs, and_mask);
> gsi = gsi_for_stmt (use_not_stmt);
> gsi_insert_before (&gsi, use_stmt, GSI_NEW_STMT);
> @@ -3344,6 +3345,8 @@ convert_atomic_bit_not (enum internal_fn fn, gimple
> *use_stmt,
> gsi_insert_after (&gsi, g, GSI_NEW_STMT);
> gsi = gsi_for_stmt (use_not_stmt);
> gsi_remove (&gsi, true);
> + gsi = gsi_for_stmt (use_stmt_removal);
> + gsi_remove (&gsi, true);
> return use_stmt;
> }
>
> @@ -3646,8 +3649,7 @@ optimize_atomic_bit_test_and (gimple_stmt_iterator
> *gsip,
> */
> }
> var = make_ssa_name (TREE_TYPE (use_rhs));
> - gsi = gsi_for_stmt (use_stmt);
> - gsi_remove (&gsi, true);
> + gimple* use_stmt_removal = use_stmt;
> g = gimple_build_assign (var, BIT_AND_EXPR, use_rhs,
> and_mask);
> gsi = gsi_for_stmt (use_nop_stmt);
> @@ -3664,6 +3666,8 @@ optimize_atomic_bit_test_and (gimple_stmt_iterator
> *gsip,
> gsi_insert_after (&gsi, g, GSI_NEW_STMT);
> gsi = gsi_for_stmt (use_nop_stmt);
> gsi_remove (&gsi, true);
> + gsi = gsi_for_stmt (use_stmt_removal);
> + gsi_remove (&gsi, true);
> }
> }
> else
> --
> 2.31.1
>
