diff mbox series

[1/2] Add tree-inlined gconds to caller cond->expr map

Message ID 20240408141815.127984-2-j@lambda.is
State New
Headers show
Series More condition coverage fixes | expand

Commit Message

Jørgen Kvalsvik April 8, 2024, 2:18 p.m. UTC
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.

	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(-)

Comments

Richard Biener April 9, 2024, 7:40 a.m. UTC | #1
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 mbox series

Patch

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