| Message ID | 20240712052329.116815-1-hongtao.liu@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix SSA_NAME leak due to def_stmt is removed before use_stmt. | expand |
On Fri, Jul 12, 2024 at 7:24 AM liuhongt <hongtao.liu@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 >
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