public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 0/2] More condition coverage fixes
@ 2024-04-08 14:18 Jørgen Kvalsvik
  2024-04-08 14:18 ` [PATCH 1/2] Add tree-inlined gconds to caller cond->expr map Jørgen Kvalsvik
  2024-04-08 14:18 ` [PATCH 2/2] Generate constant at start of loop, without UB Jørgen Kvalsvik
  0 siblings, 2 replies; 6+ messages in thread
From: Jørgen Kvalsvik @ 2024-04-08 14:18 UTC (permalink / raw)
  To: gcc-patches; +Cc: hjl.tools, rguenther, hubicka, sam, Jørgen Kvalsvik

Here are two more patches for the condition coverage.

Inlined conditions are actually counted this time, as the recorded
expression mapping uses the new, deep-copied gconds as keys and not the
pointers from the source function.

The reported UB is gone when the number of conditions are exactly 64
(sizeof uint64_t), by slightly restructuring the loop generating
constants.

This passes check-gcc RUNTESTFLAGS=gcov.exp in my ubsan instrumented
build:

configure --enable-languages=c,c++ --enable-host-shared
--enable-checking=release --disable-multilib
--with-build-config=bootstrap-ubsan

Jørgen Kvalsvik (2):
  Add tree-inlined gconds to caller cond->expr map
  Generate constant at start of loop, without UB

 gcc/testsuite/gcc.misc-tests/gcov-19.c | 37 ++++++++++++++++++++++
 gcc/tree-inline.cc                     | 43 ++++++++++++++------------
 gcc/tree-profile.cc                    | 20 +++++++++---
 3 files changed, 76 insertions(+), 24 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] Add tree-inlined gconds to caller cond->expr map
  2024-04-08 14:18 [PATCH 0/2] More condition coverage fixes Jørgen Kvalsvik
@ 2024-04-08 14:18 ` Jørgen Kvalsvik
  2024-04-09  7:40   ` Richard Biener
  2024-04-08 14:18 ` [PATCH 2/2] Generate constant at start of loop, without UB Jørgen Kvalsvik
  1 sibling, 1 reply; 6+ messages in thread
From: Jørgen Kvalsvik @ 2024-04-08 14:18 UTC (permalink / raw)
  To: gcc-patches; +Cc: hjl.tools, rguenther, hubicka, sam, Jørgen Kvalsvik

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

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
-- 
2.30.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] Generate constant at start of loop, without UB
  2024-04-08 14:18 [PATCH 0/2] More condition coverage fixes Jørgen Kvalsvik
  2024-04-08 14:18 ` [PATCH 1/2] Add tree-inlined gconds to caller cond->expr map Jørgen Kvalsvik
@ 2024-04-08 14:18 ` Jørgen Kvalsvik
  2024-04-09  7:40   ` Richard Biener
  1 sibling, 1 reply; 6+ messages in thread
From: Jørgen Kvalsvik @ 2024-04-08 14:18 UTC (permalink / raw)
  To: gcc-patches; +Cc: hjl.tools, rguenther, hubicka, sam, Jørgen Kvalsvik

Generating the constants used for recording the edges taken for
condition coverage would trigger undefined behavior when an expression
had exactly 64 (== sizeof (1ULL)) conditions, as it would generate the
constant for the next iteration at the end of the loop body, even if there
was never a next iteration. By moving the check and constant generation
to the top of the loop and hoisting the increment flag there is no
opportunity for UB.

	PR 114627

gcc/ChangeLog:

	* tree-profile.cc (instrument_decisions): Generate constant
	at the start of loop.
---
 gcc/tree-profile.cc | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/gcc/tree-profile.cc b/gcc/tree-profile.cc
index 33ff550a7bc..e58f5c83472 100644
--- a/gcc/tree-profile.cc
+++ b/gcc/tree-profile.cc
@@ -1049,6 +1049,7 @@ instrument_decisions (array_slice<basic_block> expr, size_t condno,
     zerocounter[2] = zero;
 
     unsigned xi = 0;
+    bool increment = false;
     tree rhs = build_int_cst (gcov_type_node, 1ULL << xi);
     for (basic_block current : expr)
     {
@@ -1057,7 +1058,14 @@ instrument_decisions (array_slice<basic_block> expr, size_t condno,
 	    candidates.safe_push (zerocounter);
 	counters prev = resolve_counters (candidates);
 
-	int increment = 0;
+	if (increment)
+	{
+	    xi += 1;
+	    gcc_checking_assert (xi < sizeof (uint64_t) * BITS_PER_UNIT);
+	    rhs = build_int_cst (gcov_type_node, 1ULL << xi);
+	    increment = false;
+	}
+
 	for (edge e : current->succs)
 	{
 	    counters next = prev;
@@ -1072,7 +1080,7 @@ instrument_decisions (array_slice<basic_block> expr, size_t condno,
 		    tree m = build_int_cst (gcov_type_node, masks[2*xi + k]);
 		    next[2] = emit_bitwise_op (e, prev[2], BIT_IOR_EXPR, m);
 		}
-		increment = 1;
+		increment = true;
 	    }
 	    else if (e->flags & EDGE_COMPLEX)
 	    {
@@ -1085,11 +1093,13 @@ instrument_decisions (array_slice<basic_block> expr, size_t condno,
 	    }
 	    table.get_or_insert (e->dest).safe_push (next);
 	}
-	xi += increment;
-	if (increment)
-	    rhs = build_int_cst (gcov_type_node, 1ULL << xi);
     }
 
+    /* Since this is also the return value, the number of conditions, make sure
+       to include the increment of the last basic block.  */
+    if (increment)
+	xi += 1;
+
     gcc_assert (xi == bitmap_count_bits (core));
 
     const tree relaxed = build_int_cst (integer_type_node, MEMMODEL_RELAXED);
-- 
2.30.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] Add tree-inlined gconds to caller cond->expr map
  2024-04-08 14:18 ` [PATCH 1/2] Add tree-inlined gconds to caller cond->expr map Jørgen Kvalsvik
@ 2024-04-09  7:40   ` Richard Biener
  0 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2024-04-09  7:40 UTC (permalink / raw)
  To: Jørgen Kvalsvik; +Cc: gcc-patches, hjl.tools, hubicka, sam

[-- Attachment #1: Type: text/plain, Size: 5390 bytes --]

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] Generate constant at start of loop, without UB
  2024-04-08 14:18 ` [PATCH 2/2] Generate constant at start of loop, without UB Jørgen Kvalsvik
@ 2024-04-09  7:40   ` Richard Biener
  2024-04-09  7:58     ` Jørgen Kvalsvik
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2024-04-09  7:40 UTC (permalink / raw)
  To: Jørgen Kvalsvik; +Cc: gcc-patches, hjl.tools, hubicka, sam

[-- Attachment #1: Type: text/plain, Size: 2831 bytes --]

On Mon, 8 Apr 2024, Jørgen Kvalsvik wrote:

> Generating the constants used for recording the edges taken for
> condition coverage would trigger undefined behavior when an expression
> had exactly 64 (== sizeof (1ULL)) conditions, as it would generate the
> constant for the next iteration at the end of the loop body, even if there
> was never a next iteration. By moving the check and constant generation
> to the top of the loop and hoisting the increment flag there is no
> opportunity for UB.

OK.

Thanks,
Richard.

> 	PR 114627
> 
> gcc/ChangeLog:
> 
> 	* tree-profile.cc (instrument_decisions): Generate constant
> 	at the start of loop.
> ---
>  gcc/tree-profile.cc | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/gcc/tree-profile.cc b/gcc/tree-profile.cc
> index 33ff550a7bc..e58f5c83472 100644
> --- a/gcc/tree-profile.cc
> +++ b/gcc/tree-profile.cc
> @@ -1049,6 +1049,7 @@ instrument_decisions (array_slice<basic_block> expr, size_t condno,
>      zerocounter[2] = zero;
>  
>      unsigned xi = 0;
> +    bool increment = false;
>      tree rhs = build_int_cst (gcov_type_node, 1ULL << xi);
>      for (basic_block current : expr)
>      {
> @@ -1057,7 +1058,14 @@ instrument_decisions (array_slice<basic_block> expr, size_t condno,
>  	    candidates.safe_push (zerocounter);
>  	counters prev = resolve_counters (candidates);
>  
> -	int increment = 0;
> +	if (increment)
> +	{
> +	    xi += 1;
> +	    gcc_checking_assert (xi < sizeof (uint64_t) * BITS_PER_UNIT);
> +	    rhs = build_int_cst (gcov_type_node, 1ULL << xi);
> +	    increment = false;
> +	}
> +
>  	for (edge e : current->succs)
>  	{
>  	    counters next = prev;
> @@ -1072,7 +1080,7 @@ instrument_decisions (array_slice<basic_block> expr, size_t condno,
>  		    tree m = build_int_cst (gcov_type_node, masks[2*xi + k]);
>  		    next[2] = emit_bitwise_op (e, prev[2], BIT_IOR_EXPR, m);
>  		}
> -		increment = 1;
> +		increment = true;
>  	    }
>  	    else if (e->flags & EDGE_COMPLEX)
>  	    {
> @@ -1085,11 +1093,13 @@ instrument_decisions (array_slice<basic_block> expr, size_t condno,
>  	    }
>  	    table.get_or_insert (e->dest).safe_push (next);
>  	}
> -	xi += increment;
> -	if (increment)
> -	    rhs = build_int_cst (gcov_type_node, 1ULL << xi);
>      }
>  
> +    /* Since this is also the return value, the number of conditions, make sure
> +       to include the increment of the last basic block.  */
> +    if (increment)
> +	xi += 1;
> +
>      gcc_assert (xi == bitmap_count_bits (core));
>  
>      const tree relaxed = build_int_cst (integer_type_node, MEMMODEL_RELAXED);
> 

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] Generate constant at start of loop, without UB
  2024-04-09  7:40   ` Richard Biener
@ 2024-04-09  7:58     ` Jørgen Kvalsvik
  0 siblings, 0 replies; 6+ messages in thread
From: Jørgen Kvalsvik @ 2024-04-09  7:58 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches, hjl.tools, hubicka, sam

On 09/04/2024 09:40, Richard Biener wrote:
> On Mon, 8 Apr 2024, Jørgen Kvalsvik wrote:
> 
>> Generating the constants used for recording the edges taken for
>> condition coverage would trigger undefined behavior when an expression
>> had exactly 64 (== sizeof (1ULL)) conditions, as it would generate the
>> constant for the next iteration at the end of the loop body, even if there
>> was never a next iteration. By moving the check and constant generation
>> to the top of the loop and hoisting the increment flag there is no
>> opportunity for UB.
> 
> OK.
> 
> Thanks,
> Richard.

Thanks, committed.

> 
>> 	PR 114627
>>
>> gcc/ChangeLog:
>>
>> 	* tree-profile.cc (instrument_decisions): Generate constant
>> 	at the start of loop.
>> ---
>>   gcc/tree-profile.cc | 20 +++++++++++++++-----
>>   1 file changed, 15 insertions(+), 5 deletions(-)
>>
>> diff --git a/gcc/tree-profile.cc b/gcc/tree-profile.cc
>> index 33ff550a7bc..e58f5c83472 100644
>> --- a/gcc/tree-profile.cc
>> +++ b/gcc/tree-profile.cc
>> @@ -1049,6 +1049,7 @@ instrument_decisions (array_slice<basic_block> expr, size_t condno,
>>       zerocounter[2] = zero;
>>   
>>       unsigned xi = 0;
>> +    bool increment = false;
>>       tree rhs = build_int_cst (gcov_type_node, 1ULL << xi);
>>       for (basic_block current : expr)
>>       {
>> @@ -1057,7 +1058,14 @@ instrument_decisions (array_slice<basic_block> expr, size_t condno,
>>   	    candidates.safe_push (zerocounter);
>>   	counters prev = resolve_counters (candidates);
>>   
>> -	int increment = 0;
>> +	if (increment)
>> +	{
>> +	    xi += 1;
>> +	    gcc_checking_assert (xi < sizeof (uint64_t) * BITS_PER_UNIT);
>> +	    rhs = build_int_cst (gcov_type_node, 1ULL << xi);
>> +	    increment = false;
>> +	}
>> +
>>   	for (edge e : current->succs)
>>   	{
>>   	    counters next = prev;
>> @@ -1072,7 +1080,7 @@ instrument_decisions (array_slice<basic_block> expr, size_t condno,
>>   		    tree m = build_int_cst (gcov_type_node, masks[2*xi + k]);
>>   		    next[2] = emit_bitwise_op (e, prev[2], BIT_IOR_EXPR, m);
>>   		}
>> -		increment = 1;
>> +		increment = true;
>>   	    }
>>   	    else if (e->flags & EDGE_COMPLEX)
>>   	    {
>> @@ -1085,11 +1093,13 @@ instrument_decisions (array_slice<basic_block> expr, size_t condno,
>>   	    }
>>   	    table.get_or_insert (e->dest).safe_push (next);
>>   	}
>> -	xi += increment;
>> -	if (increment)
>> -	    rhs = build_int_cst (gcov_type_node, 1ULL << xi);
>>       }
>>   
>> +    /* Since this is also the return value, the number of conditions, make sure
>> +       to include the increment of the last basic block.  */
>> +    if (increment)
>> +	xi += 1;
>> +
>>       gcc_assert (xi == bitmap_count_bits (core));
>>   
>>       const tree relaxed = build_int_cst (integer_type_node, MEMMODEL_RELAXED);
>>
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-04-09  7:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-08 14:18 [PATCH 0/2] More condition coverage fixes Jørgen Kvalsvik
2024-04-08 14:18 ` [PATCH 1/2] Add tree-inlined gconds to caller cond->expr map Jørgen Kvalsvik
2024-04-09  7:40   ` Richard Biener
2024-04-08 14:18 ` [PATCH 2/2] Generate constant at start of loop, without UB Jørgen Kvalsvik
2024-04-09  7:40   ` Richard Biener
2024-04-09  7:58     ` Jørgen Kvalsvik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).