Message ID | 20240408141815.127984-2-j@lambda.is |
---|---|
State | New |
Headers | show |
Series | More condition coverage fixes | expand |
On Mon, 8 Apr 2024, Jørgen Kvalsvik wrote: > Properly add the condition -> expression mapping of inlined gconds from > the caller into the callee map. This is a fix for PR114599 that works > beyond fixing the segfault, as the previous fixed copied references to > the source gconds, not the deep copied ones that end up in the calle > body. > > The new tests checks this, both in the case of a calle without > conditions (which triggered the segfault), and a test that shows that > conditions are properly mapped, and not mixed. OK. Thanks, Richard. > PR middle-end/114599 > > gcc/ChangeLog: > > * tree-inline.cc (copy_bb): Copy cond_uids into callee. > (prepend_lexical_block): Remove outdated comment. > (add_local_variables): Remove bad cond_uids copy. > > gcc/testsuite/ChangeLog: > > * gcc.misc-tests/gcov-19.c: New test. > --- > gcc/testsuite/gcc.misc-tests/gcov-19.c | 37 ++++++++++++++++++++++ > gcc/tree-inline.cc | 43 ++++++++++++++------------ > 2 files changed, 61 insertions(+), 19 deletions(-) > > diff --git a/gcc/testsuite/gcc.misc-tests/gcov-19.c b/gcc/testsuite/gcc.misc-tests/gcov-19.c > index b83a38531ba..78769fa57b8 100644 > --- a/gcc/testsuite/gcc.misc-tests/gcov-19.c > +++ b/gcc/testsuite/gcc.misc-tests/gcov-19.c > @@ -1468,6 +1468,40 @@ mcdc026e (int a, int b, int c, int d, int e) > return x + y; > } > > +__attribute__((always_inline)) > +inline int > +mcdc027_inlined (int a) > +{ > + if (a) > + x = a; > + else > + x = 1; > + > + return a + 1; > +} > + > +/* Check that conditions in a function inlined into a function without > + conditionals works. */ > +void > +mcdc027a (int a) /* conditions(1/2) false(0) */ > + /* conditions(end) */ > +{ > + mcdc027_inlined (a); > +} > + > +/* Check that conditions in a function inlined into a function with > + conditionals works and that the conditions are not mixed up. */ > +void > +mcdc027b (int a) /* conditions(1/2) false(0) */ > + /* conditions(end) */ > +{ > + int v = mcdc027_inlined (a); > + > + if (v > 10) /* conditions(1/2) true(0) */ > + /* condiitions(end) */ > + x = 10; > +} > + > int main () > { > mcdc001a (0, 1); > @@ -1721,6 +1755,9 @@ int main () > mcdc026e (1, 1, 1, 0, 1); > mcdc026e (1, 1, 0, 0, 1); > mcdc026e (1, 1, 1, 1, 1); > + > + mcdc027a (1); > + mcdc027b (1); > } > > /* { dg-final { run-gcov conditions { --conditions gcov-19.c } } } */ > diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc > index b18917707cc..5f852885e7f 100644 > --- a/gcc/tree-inline.cc > +++ b/gcc/tree-inline.cc > @@ -2049,6 +2049,17 @@ copy_bb (copy_body_data *id, basic_block bb, > > copy_gsi = gsi_start_bb (copy_basic_block); > > + unsigned min_cond_uid = 0; > + if (id->src_cfun->cond_uids) > + { > + if (!cfun->cond_uids) > + cfun->cond_uids = new hash_map <gcond*, unsigned> (); > + > + for (auto itr : *id->src_cfun->cond_uids) > + if (itr.second >= min_cond_uid) > + min_cond_uid = itr.second + 1; > + } > + > for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) > { > gimple_seq stmts; > @@ -2076,6 +2087,18 @@ copy_bb (copy_body_data *id, basic_block bb, > if (gimple_nop_p (stmt)) > continue; > > + /* If -fcondition-coverage is used, register the inlined conditions > + in the cond->expression mapping of the caller. The expression tag > + is shifted conditions from the two bodies are not mixed. */ > + if (id->src_cfun->cond_uids && is_a <gcond*> (orig_stmt)) > + { > + gcond *orig_cond = as_a <gcond*> (orig_stmt); > + gcond *cond = as_a <gcond*> (stmt); > + unsigned *v = id->src_cfun->cond_uids->get (orig_cond); > + if (v) > + cfun->cond_uids->put (cond, *v + min_cond_uid); > + } > + > gimple_duplicate_stmt_histograms (cfun, stmt, id->src_cfun, > orig_stmt); > > @@ -4659,8 +4682,7 @@ prepend_lexical_block (tree current_block, tree new_block) > BLOCK_SUPERCONTEXT (new_block) = current_block; > } > > -/* Add local variables from CALLEE to CALLER. If set for condition coverage, > - copy basic condition -> expression mapping to CALLER. */ > +/* Add local variables from CALLEE to CALLER. */ > > static inline void > add_local_variables (struct function *callee, struct function *caller, > @@ -4690,23 +4712,6 @@ add_local_variables (struct function *callee, struct function *caller, > } > add_local_decl (caller, new_var); > } > - > - /* If -fcondition-coverage is used and the caller has conditions, copy the > - mapping into the caller but and the end so the caller and callee > - expressions aren't mixed. */ > - if (callee->cond_uids) > - { > - if (!caller->cond_uids) > - caller->cond_uids = new hash_map <gcond*, unsigned> (); > - > - unsigned dst_max_uid = 0; > - for (auto itr : *callee->cond_uids) > - if (itr.second >= dst_max_uid) > - dst_max_uid = itr.second + 1; > - > - for (auto itr : *callee->cond_uids) > - caller->cond_uids->put (itr.first, itr.second + dst_max_uid); > - } > } > > /* Add to BINDINGS a debug stmt resetting SRCVAR if inlining might >
diff --git a/gcc/testsuite/gcc.misc-tests/gcov-19.c b/gcc/testsuite/gcc.misc-tests/gcov-19.c index b83a38531ba..78769fa57b8 100644 --- a/gcc/testsuite/gcc.misc-tests/gcov-19.c +++ b/gcc/testsuite/gcc.misc-tests/gcov-19.c @@ -1468,6 +1468,40 @@ mcdc026e (int a, int b, int c, int d, int e) return x + y; } +__attribute__((always_inline)) +inline int +mcdc027_inlined (int a) +{ + if (a) + x = a; + else + x = 1; + + return a + 1; +} + +/* Check that conditions in a function inlined into a function without + conditionals works. */ +void +mcdc027a (int a) /* conditions(1/2) false(0) */ + /* conditions(end) */ +{ + mcdc027_inlined (a); +} + +/* Check that conditions in a function inlined into a function with + conditionals works and that the conditions are not mixed up. */ +void +mcdc027b (int a) /* conditions(1/2) false(0) */ + /* conditions(end) */ +{ + int v = mcdc027_inlined (a); + + if (v > 10) /* conditions(1/2) true(0) */ + /* condiitions(end) */ + x = 10; +} + int main () { mcdc001a (0, 1); @@ -1721,6 +1755,9 @@ int main () mcdc026e (1, 1, 1, 0, 1); mcdc026e (1, 1, 0, 0, 1); mcdc026e (1, 1, 1, 1, 1); + + mcdc027a (1); + mcdc027b (1); } /* { dg-final { run-gcov conditions { --conditions gcov-19.c } } } */ diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc index b18917707cc..5f852885e7f 100644 --- a/gcc/tree-inline.cc +++ b/gcc/tree-inline.cc @@ -2049,6 +2049,17 @@ copy_bb (copy_body_data *id, basic_block bb, copy_gsi = gsi_start_bb (copy_basic_block); + unsigned min_cond_uid = 0; + if (id->src_cfun->cond_uids) + { + if (!cfun->cond_uids) + cfun->cond_uids = new hash_map <gcond*, unsigned> (); + + for (auto itr : *id->src_cfun->cond_uids) + if (itr.second >= min_cond_uid) + min_cond_uid = itr.second + 1; + } + for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) { gimple_seq stmts; @@ -2076,6 +2087,18 @@ copy_bb (copy_body_data *id, basic_block bb, if (gimple_nop_p (stmt)) continue; + /* If -fcondition-coverage is used, register the inlined conditions + in the cond->expression mapping of the caller. The expression tag + is shifted conditions from the two bodies are not mixed. */ + if (id->src_cfun->cond_uids && is_a <gcond*> (orig_stmt)) + { + gcond *orig_cond = as_a <gcond*> (orig_stmt); + gcond *cond = as_a <gcond*> (stmt); + unsigned *v = id->src_cfun->cond_uids->get (orig_cond); + if (v) + cfun->cond_uids->put (cond, *v + min_cond_uid); + } + gimple_duplicate_stmt_histograms (cfun, stmt, id->src_cfun, orig_stmt); @@ -4659,8 +4682,7 @@ prepend_lexical_block (tree current_block, tree new_block) BLOCK_SUPERCONTEXT (new_block) = current_block; } -/* Add local variables from CALLEE to CALLER. If set for condition coverage, - copy basic condition -> expression mapping to CALLER. */ +/* Add local variables from CALLEE to CALLER. */ static inline void add_local_variables (struct function *callee, struct function *caller, @@ -4690,23 +4712,6 @@ add_local_variables (struct function *callee, struct function *caller, } add_local_decl (caller, new_var); } - - /* If -fcondition-coverage is used and the caller has conditions, copy the - mapping into the caller but and the end so the caller and callee - expressions aren't mixed. */ - if (callee->cond_uids) - { - if (!caller->cond_uids) - caller->cond_uids = new hash_map <gcond*, unsigned> (); - - unsigned dst_max_uid = 0; - for (auto itr : *callee->cond_uids) - if (itr.second >= dst_max_uid) - dst_max_uid = itr.second + 1; - - for (auto itr : *callee->cond_uids) - caller->cond_uids->put (itr.first, itr.second + dst_max_uid); - } } /* Add to BINDINGS a debug stmt resetting SRCVAR if inlining might