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
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
