[PATCH] Fix count comparison in ipa-cp

[PATCH] Fix count comparison in ipa-cp

[Eugene Rozenfeld]

The existing comparison was incorrect for non-PRECISE counts
(e.g., AFDO): we could end up with a 0 base_count, which could
lead to asserts, e.g., in good_cloning_opportunity_p.

gcc/ChangeLog:

        * ipa-cp.cc (ipcp_propagate_stage): Fix profile count comparison.
---
 gcc/ipa-cp.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc
index d2bcd5e5e69..9df8b456759 100644
--- a/gcc/ipa-cp.cc
+++ b/gcc/ipa-cp.cc
@@ -4225,7 +4225,7 @@ ipcp_propagate_stage (class ipa_topo_info *topo)
 	for (cgraph_edge *cs = node->callees; cs; cs = cs->next_callee)
 	  {
 	    profile_count count = cs->count.ipa ();
-	    if (!(count > profile_count::zero ()))
+	    if (!count.nonzero_p ())
 	      continue;
 
 	    enum availability avail;
-- 
2.25.1

Re: [PATCH] Fix count comparison in ipa-cp

[Jeff Law]

On 11/21/22 14:26, Eugene Rozenfeld via Gcc-patches wrote:
> The existing comparison was incorrect for non-PRECISE counts
> (e.g., AFDO): we could end up with a 0 base_count, which could
> lead to asserts, e.g., in good_cloning_opportunity_p.
>
> gcc/ChangeLog:
>
>          * ipa-cp.cc (ipcp_propagate_stage): Fix profile count comparison.

OK.  Probably somewhat painful to pull together a reliable test for 
this, right?


Jeff

RE: [EXTERNAL] Re: [PATCH] Fix count comparison in ipa-cp

[Eugene Rozenfeld]

I initially ran into this while reviving autoprofiledbootstrap build.

I was able to create a simple reliable test for this bug and captured it in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108000

I also included the test case in the updated patch below.

Eugene

=================================

The existing comparison was incorrect for non-PRECISE counts
(e.g., AFDO): we could end up with a 0 base_count, which could
lead to asserts, e.g., in good_cloning_opportunity_p.

Tested on x86_64-pc-linux-gnu.

gcc/ChangeLog:
	PR ipa/108000
	* ipa-cp.cc (ipcp_propagate_stage): Fix profile count comparison

gcc/testsuite
	* gcc.dg/tree-prof/pr108000.c: Regression test
---
 gcc/ipa-cp.cc                             |  2 +-
 gcc/testsuite/gcc.dg/tree-prof/pr108000.c | 93 +++++++++++++++++++++++
 2 files changed, 94 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-prof/pr108000.c

diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc
index d5230c7c5e6..cc031ebed0f 100644
--- a/gcc/ipa-cp.cc
+++ b/gcc/ipa-cp.cc
@@ -4225,7 +4225,7 @@ ipcp_propagate_stage (class ipa_topo_info *topo)
 	for (cgraph_edge *cs = node->callees; cs; cs = cs->next_callee)
 	  {
 	    profile_count count = cs->count.ipa ();
-	    if (!(count > profile_count::zero ()))
+	    if (!count.nonzero_p ())
 	      continue;
 
 	    enum availability avail;
diff --git a/gcc/testsuite/gcc.dg/tree-prof/pr108000.c b/gcc/testsuite/gcc.dg/tree-prof/pr108000.c
new file mode 100644
index 00000000000..c59ea799748
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-prof/pr108000.c
@@ -0,0 +1,93 @@
+/* { dg-options "-O2" } */
+
+#include <stdlib.h>
+
+volatile int flag;
+const int array_size = 10;
+int* array;
+int iterations = 10000000;
+
+#define BAR(num) \
+int __attribute__((noinline)) \
+bar##num (int i, int j) \
+{ \
+  if (i == 0) \
+    return 2*num - 1; \
+  else \
+    return 2*num; \
+}
+
+BAR(1)
+BAR(2)
+BAR(3)
+BAR(4)
+BAR(5)
+BAR(6)
+BAR(7)
+BAR(8)
+BAR(9)
+BAR(10)
+BAR(11)
+BAR(12)
+BAR(13)
+BAR(14)
+BAR(15)
+BAR(16)
+BAR(17)
+BAR(18)
+BAR(19)
+
+int __attribute__((noinline))
+foo ()
+{
+  switch (flag)
+  {
+	case 1:
+		return bar1 (0, 0);
+	case 2:
+		return bar2 (0, 0);
+	case 3:
+		return bar3 (0, 0);
+	case 4:
+		return bar4 (0, 0);
+	case 5:
+		return bar5 (0, 0);
+	case 6:
+		return bar6 (0, 0);
+	case 7:
+		return bar7 (0, 0);
+	case 8:
+		return bar8 (0, 0);
+	case 9:
+		return bar9 (0, 0);
+	case 10:
+		return bar10 (0, 0);
+	case 11:
+		return bar11 (0, 0);
+	case 12:
+		return bar12 (0, 0);
+	case 13:
+		return bar13 (0, 0);
+	case 14:
+		return bar14 (0, 0);
+	case 15:
+		return bar15 (0, 0);
+	case 16:
+		return bar16 (0, 0);
+	case 17:
+		return bar17 (0, 0);
+	case 18:
+		return bar18 (0, 0);
+	default:
+		return bar19(0, 0);
+  }
+}
+
+int
+main ()
+{
+  flag = 0;
+  array = calloc(array_size, sizeof(int));
+  for (int i = 0, j = 0; i < iterations; ++i, j = (j + 1) % 10)
+    array[j] = foo ();
+}
-- 
2.25.1

-----Original Message-----
From: Jeff Law <jeffreyalaw@gmail.com> 
Sent: Tuesday, November 22, 2022 12:03 PM
To: Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com>; gcc-patches@gcc.gnu.org
Subject: [EXTERNAL] Re: [PATCH] Fix count comparison in ipa-cp

[You don't often get email from jeffreyalaw@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]

On 11/21/22 14:26, Eugene Rozenfeld via Gcc-patches wrote:
> The existing comparison was incorrect for non-PRECISE counts (e.g., 
> AFDO): we could end up with a 0 base_count, which could lead to 
> asserts, e.g., in good_cloning_opportunity_p.
>
> gcc/ChangeLog:
>
>          * ipa-cp.cc (ipcp_propagate_stage): Fix profile count comparison.

OK.  Probably somewhat painful to pull together a reliable test for this, right?


Jeff