public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] ipa-utils: avoid generating uninitialized probabilities on merges.
@ 2023-09-27 14:44 Sergei Trofimovich
  2023-10-01 19:29 ` [PATCH v2] ipa-utils: avoid uninitialized probabilities on ICF [PR111559] Sergei Trofimovich
  0 siblings, 1 reply; 7+ messages in thread
From: Sergei Trofimovich @ 2023-09-27 14:44 UTC (permalink / raw)
  To: gcc-patches, Jan Hubicka, Richard Biener; +Cc: Sergei Trofimovich

From: Sergei Trofimovich <siarheit@google.com>

r14-3459-g0c78240fd7d519 "Check that passes do not forget to define profile"
exposed check failures in cases when gcc produces uninitialized profile
probabilities. In case of PR/111559 uninitialized profile is generated
by edges executed 0 times during profile:

    __attribute__((noipa)) static void edge(void) {}

    int p = 0;

    __attribute__((noinline))
    static void rule1(void) { if (p) edge(); }

    __attribute__((noinline))
    static void rule1_same(void) { if (p) edge(); }

    __attribute__((noipa)) int main(void) {
        rule1();
        rule1_same();
    }

    $ gcc -O2 -fprofile-generate bug.c -o b -fopt-info
    $ ./b
    $ gcc -O2 -fprofile-use -fprofile-correction bug.c -o b -fopt-info

    bug.c: In function 'rule1':
    bug.c:6:13: error: probability of edge 3->4 not initialized
        6 | static void rule1(void) { if (p) edge(); }
          |             ^~~~~
    during GIMPLE pass: fixup_cfg
    bug.c:6:13: internal compiler error: verify_flow_info failed

The change conservatively ignores updates with uninitialized values and
uses initially assigned probabilities (`always` probability in case of
the example).

gcc/
	PR/111283
	PR/111559
	* ipa-utils.cc (ipa_merge_profiles): Avoid producing
	  uninitialized probabilities when merging counters with zero
	  denominators.
---
 gcc/ipa-utils.cc | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/gcc/ipa-utils.cc b/gcc/ipa-utils.cc
index 956c6294fd7..7c53ae9dd45 100644
--- a/gcc/ipa-utils.cc
+++ b/gcc/ipa-utils.cc
@@ -651,13 +651,17 @@ ipa_merge_profiles (struct cgraph_node *dst,
 		{
 		  edge srce = EDGE_SUCC (srcbb, i);
 		  edge dste = EDGE_SUCC (dstbb, i);
-		  dste->probability = 
+		  profile_probability merged =
 		    dste->probability * dstbb->count.ipa ().probability_in
 						 (dstbb->count.ipa ()
 						  + srccount.ipa ())
 		    + srce->probability * srcbb->count.ipa ().probability_in
 						 (dstbb->count.ipa ()
 						  + srccount.ipa ());
+		  /* We produce uninitialized probabilities when
+		     denominator is zero: https://gcc.gnu.org/PR111559.  */
+		  if (merged.initialized_p ())
+		    dste->probability = merged;
 		}
 	      dstbb->count = dstbb->count.ipa () + srccount.ipa ();
 	    }
-- 
2.42.0


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

* [PATCH v2] ipa-utils: avoid uninitialized probabilities on ICF [PR111559]
  2023-09-27 14:44 [PATCH] ipa-utils: avoid generating uninitialized probabilities on merges Sergei Trofimovich
@ 2023-10-01 19:29 ` Sergei Trofimovich
  2023-10-05 11:52   ` Jan Hubicka
  0 siblings, 1 reply; 7+ messages in thread
From: Sergei Trofimovich @ 2023-10-01 19:29 UTC (permalink / raw)
  To: gcc-patches; +Cc: slyich, hubicka, rguenther, siarheit

From: Sergei Trofimovich <siarheit@google.com>

r14-3459-g0c78240fd7d519 "Check that passes do not forget to define profile"
exposed check failures in cases when gcc produces uninitialized profile
probabilities. In case of PR/111559 uninitialized profile is generated
by edges executed 0 times reported by IPA profile:

    $ gcc -O2 -fprofile-generate pr111559.c -o b -fopt-info
    $ ./b
    $ gcc -O2 -fprofile-use -fprofile-correction pr111559.c -o b -fopt-info

    pr111559.c: In function 'rule1':
    pr111559.c:6:13: error: probability of edge 3->4 not initialized
        6 | static void rule1(void) { if (p) edge(); }
          |             ^~~~~
    during GIMPLE pass: fixup_cfg
    pr111559.c:6:13: internal compiler error: verify_flow_info failed

The change conservatively ignores updates with uninitialized values and
uses initially assigned probabilities (`always` probability in case of
the example).

	PR ipa/111283
	PR gcov-profile/111559

gcc/
	* ipa-utils.cc (ipa_merge_profiles): Avoid producing
	uninitialized probabilities when merging counters with zero
	denominators.

gcc/testsuite/
	* gcc.dg/tree-prof/pr111559.c: New test.
---
 gcc/ipa-utils.cc                          |  6 +++++-
 gcc/testsuite/gcc.dg/tree-prof/pr111559.c | 16 ++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-prof/pr111559.c

diff --git a/gcc/ipa-utils.cc b/gcc/ipa-utils.cc
index 956c6294fd7..7c53ae9dd45 100644
--- a/gcc/ipa-utils.cc
+++ b/gcc/ipa-utils.cc
@@ -651,13 +651,17 @@ ipa_merge_profiles (struct cgraph_node *dst,
 		{
 		  edge srce = EDGE_SUCC (srcbb, i);
 		  edge dste = EDGE_SUCC (dstbb, i);
-		  dste->probability = 
+		  profile_probability merged =
 		    dste->probability * dstbb->count.ipa ().probability_in
 						 (dstbb->count.ipa ()
 						  + srccount.ipa ())
 		    + srce->probability * srcbb->count.ipa ().probability_in
 						 (dstbb->count.ipa ()
 						  + srccount.ipa ());
+		  /* We produce uninitialized probabilities when
+		     denominator is zero: https://gcc.gnu.org/PR111559.  */
+		  if (merged.initialized_p ())
+		    dste->probability = merged;
 		}
 	      dstbb->count = dstbb->count.ipa () + srccount.ipa ();
 	    }
diff --git a/gcc/testsuite/gcc.dg/tree-prof/pr111559.c b/gcc/testsuite/gcc.dg/tree-prof/pr111559.c
new file mode 100644
index 00000000000..43202c6c888
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-prof/pr111559.c
@@ -0,0 +1,16 @@
+/* { dg-options "-O2" } */
+
+__attribute__((noipa)) static void edge(void) {}
+
+int p = 0;
+
+__attribute__((noinline))
+static void rule1(void) { if (p) edge(); }
+
+__attribute__((noinline))
+static void rule1_same(void) { if (p) edge(); }
+
+__attribute__((noipa)) int main(void) {
+    rule1();
+    rule1_same();
+}
-- 
2.42.0


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

* Re: [PATCH v2] ipa-utils: avoid uninitialized probabilities on ICF [PR111559]
  2023-10-01 19:29 ` [PATCH v2] ipa-utils: avoid uninitialized probabilities on ICF [PR111559] Sergei Trofimovich
@ 2023-10-05 11:52   ` Jan Hubicka
  2023-10-05 12:46     ` Sergei Trofimovich
  0 siblings, 1 reply; 7+ messages in thread
From: Jan Hubicka @ 2023-10-05 11:52 UTC (permalink / raw)
  To: Sergei Trofimovich; +Cc: gcc-patches, rguenther, siarheit

> From: Sergei Trofimovich <siarheit@google.com>
> 
> r14-3459-g0c78240fd7d519 "Check that passes do not forget to define profile"
> exposed check failures in cases when gcc produces uninitialized profile
> probabilities. In case of PR/111559 uninitialized profile is generated
> by edges executed 0 times reported by IPA profile:
> 
>     $ gcc -O2 -fprofile-generate pr111559.c -o b -fopt-info
>     $ ./b
>     $ gcc -O2 -fprofile-use -fprofile-correction pr111559.c -o b -fopt-info
> 
>     pr111559.c: In function 'rule1':
>     pr111559.c:6:13: error: probability of edge 3->4 not initialized
>         6 | static void rule1(void) { if (p) edge(); }
>           |             ^~~~~
>     during GIMPLE pass: fixup_cfg
>     pr111559.c:6:13: internal compiler error: verify_flow_info failed
> 
> The change conservatively ignores updates with uninitialized values and
> uses initially assigned probabilities (`always` probability in case of
> the example).
> 
> 	PR ipa/111283
> 	PR gcov-profile/111559
> 
> gcc/
> 	* ipa-utils.cc (ipa_merge_profiles): Avoid producing
> 	uninitialized probabilities when merging counters with zero
> 	denominators.
> 
> gcc/testsuite/
> 	* gcc.dg/tree-prof/pr111559.c: New test.
> ---
>  gcc/ipa-utils.cc                          |  6 +++++-
>  gcc/testsuite/gcc.dg/tree-prof/pr111559.c | 16 ++++++++++++++++
>  2 files changed, 21 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.dg/tree-prof/pr111559.c
> 
> diff --git a/gcc/ipa-utils.cc b/gcc/ipa-utils.cc
> index 956c6294fd7..7c53ae9dd45 100644
> --- a/gcc/ipa-utils.cc
> +++ b/gcc/ipa-utils.cc
> @@ -651,13 +651,17 @@ ipa_merge_profiles (struct cgraph_node *dst,
>  		{
>  		  edge srce = EDGE_SUCC (srcbb, i);
>  		  edge dste = EDGE_SUCC (dstbb, i);
> -		  dste->probability = 
> +		  profile_probability merged =
>  		    dste->probability * dstbb->count.ipa ().probability_in
>  						 (dstbb->count.ipa ()
>  						  + srccount.ipa ())
>  		    + srce->probability * srcbb->count.ipa ().probability_in
>  						 (dstbb->count.ipa ()
>  						  + srccount.ipa ());
> +		  /* We produce uninitialized probabilities when
> +		     denominator is zero: https://gcc.gnu.org/PR111559.  */
> +		  if (merged.initialized_p ())
> +		    dste->probability = merged;

Thanks for the patch.
We usually avoid the uninitialized value here by simply checking that
parameter of probability_in satifies nonzero_p.  So I think it would be
more consistent doing it here to:

  profile_probability sum = dstbb->count.ipa () + srccount.ipa ()
  if (sum.nonzero_p ())
  {
     dste->probability = .....
  }

OK with this change.
Honza
>  		}
>  	      dstbb->count = dstbb->count.ipa () + srccount.ipa ();
>  	    }
> diff --git a/gcc/testsuite/gcc.dg/tree-prof/pr111559.c b/gcc/testsuite/gcc.dg/tree-prof/pr111559.c
> new file mode 100644
> index 00000000000..43202c6c888
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-prof/pr111559.c
> @@ -0,0 +1,16 @@
> +/* { dg-options "-O2" } */
> +
> +__attribute__((noipa)) static void edge(void) {}
> +
> +int p = 0;
> +
> +__attribute__((noinline))
> +static void rule1(void) { if (p) edge(); }
> +
> +__attribute__((noinline))
> +static void rule1_same(void) { if (p) edge(); }
> +
> +__attribute__((noipa)) int main(void) {
> +    rule1();
> +    rule1_same();
> +}
> -- 
> 2.42.0
> 

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

* Re: [PATCH v2] ipa-utils: avoid uninitialized probabilities on ICF [PR111559]
  2023-10-05 11:52   ` Jan Hubicka
@ 2023-10-05 12:46     ` Sergei Trofimovich
  2023-10-05 13:04       ` Jan Hubicka
  0 siblings, 1 reply; 7+ messages in thread
From: Sergei Trofimovich @ 2023-10-05 12:46 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: gcc-patches, rguenther, siarheit

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

On Thu, Oct 05, 2023 at 01:52:30PM +0200, Jan Hubicka wrote:
> > From: Sergei Trofimovich <siarheit@google.com>
> > 
> > r14-3459-g0c78240fd7d519 "Check that passes do not forget to define profile"
> > exposed check failures in cases when gcc produces uninitialized profile
> > probabilities. In case of PR/111559 uninitialized profile is generated
> > by edges executed 0 times reported by IPA profile:
> > 
> >     $ gcc -O2 -fprofile-generate pr111559.c -o b -fopt-info
> >     $ ./b
> >     $ gcc -O2 -fprofile-use -fprofile-correction pr111559.c -o b -fopt-info
> > 
> >     pr111559.c: In function 'rule1':
> >     pr111559.c:6:13: error: probability of edge 3->4 not initialized
> >         6 | static void rule1(void) { if (p) edge(); }
> >           |             ^~~~~
> >     during GIMPLE pass: fixup_cfg
> >     pr111559.c:6:13: internal compiler error: verify_flow_info failed
> > 
> > The change conservatively ignores updates with uninitialized values and
> > uses initially assigned probabilities (`always` probability in case of
> > the example).
> > 
> > 	PR ipa/111283
> > 	PR gcov-profile/111559
> > 
> > gcc/
> > 	* ipa-utils.cc (ipa_merge_profiles): Avoid producing
> > 	uninitialized probabilities when merging counters with zero
> > 	denominators.
> > 
> > gcc/testsuite/
> > 	* gcc.dg/tree-prof/pr111559.c: New test.
> > ---
> >  gcc/ipa-utils.cc                          |  6 +++++-
> >  gcc/testsuite/gcc.dg/tree-prof/pr111559.c | 16 ++++++++++++++++
> >  2 files changed, 21 insertions(+), 1 deletion(-)
> >  create mode 100644 gcc/testsuite/gcc.dg/tree-prof/pr111559.c
> > 
> > diff --git a/gcc/ipa-utils.cc b/gcc/ipa-utils.cc
> > index 956c6294fd7..7c53ae9dd45 100644
> > --- a/gcc/ipa-utils.cc
> > +++ b/gcc/ipa-utils.cc
> > @@ -651,13 +651,17 @@ ipa_merge_profiles (struct cgraph_node *dst,
> >  		{
> >  		  edge srce = EDGE_SUCC (srcbb, i);
> >  		  edge dste = EDGE_SUCC (dstbb, i);
> > -		  dste->probability = 
> > +		  profile_probability merged =
> >  		    dste->probability * dstbb->count.ipa ().probability_in
> >  						 (dstbb->count.ipa ()
> >  						  + srccount.ipa ())
> >  		    + srce->probability * srcbb->count.ipa ().probability_in
> >  						 (dstbb->count.ipa ()
> >  						  + srccount.ipa ());
> > +		  /* We produce uninitialized probabilities when
> > +		     denominator is zero: https://gcc.gnu.org/PR111559.  */
> > +		  if (merged.initialized_p ())
> > +		    dste->probability = merged;
> 
> Thanks for the patch.
> We usually avoid the uninitialized value here by simply checking that
> parameter of probability_in satifies nonzero_p.  So I think it would be
> more consistent doing it here to:
> 
>   profile_probability sum = dstbb->count.ipa () + srccount.ipa ()
>   if (sum.nonzero_p ())
>   {
>      dste->probability = .....
>   }

Aha, sounds good! I had to do `s/profile_probability/profile_count` as
it's a denominator value for probability.

Attached v3 just in case.

> OK with this change.
> Honza
> >  		}
> >  	      dstbb->count = dstbb->count.ipa () + srccount.ipa ();
> >  	    }
> > diff --git a/gcc/testsuite/gcc.dg/tree-prof/pr111559.c b/gcc/testsuite/gcc.dg/tree-prof/pr111559.c
> > new file mode 100644
> > index 00000000000..43202c6c888
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/tree-prof/pr111559.c
> > @@ -0,0 +1,16 @@
> > +/* { dg-options "-O2" } */
> > +
> > +__attribute__((noipa)) static void edge(void) {}
> > +
> > +int p = 0;
> > +
> > +__attribute__((noinline))
> > +static void rule1(void) { if (p) edge(); }
> > +
> > +__attribute__((noinline))
> > +static void rule1_same(void) { if (p) edge(); }
> > +
> > +__attribute__((noipa)) int main(void) {
> > +    rule1();
> > +    rule1_same();
> > +}
> > -- 
> > 2.42.0
> > 

-- 

  Sergei

[-- Attachment #2: v3-0001-ipa-utils-avoid-uninitialized-probabilities-on-IC.patch --]
[-- Type: text/plain, Size: 3166 bytes --]

From 97122ebae5a7ed43b6c31574c761a54bee3a96ec Mon Sep 17 00:00:00 2001
From: Sergei Trofimovich <siarheit@google.com>
Date: Wed, 27 Sep 2023 14:29:12 +0100
Subject: [PATCH v3] ipa-utils: avoid uninitialized probabilities on ICF
 [PR111559]

r14-3459-g0c78240fd7d519 "Check that passes do not forget to define profile"
exposed check failures in cases when gcc produces uninitialized profile
probabilities. In case of PR/111559 uninitialized profile is generated
by edges executed 0 times reported by IPA profile:

    $ gcc -O2 -fprofile-generate pr111559.c -o b -fopt-info
    $ ./b
    $ gcc -O2 -fprofile-use -fprofile-correction pr111559.c -o b -fopt-info

    pr111559.c: In function 'rule1':
    pr111559.c:6:13: error: probability of edge 3->4 not initialized
        6 | static void rule1(void) { if (p) edge(); }
          |             ^~~~~
    during GIMPLE pass: fixup_cfg
    pr111559.c:6:13: internal compiler error: verify_flow_info failed

The change conservatively ignores updates with zero execution counts and
uses initially assigned probabilities (`always` probability in case of
the example).

	PR ipa/111283
	PR gcov-profile/111559

gcc/
	* ipa-utils.cc (ipa_merge_profiles): Avoid producing
	uninitialized probabilities when merging counters with zero
	denominators.

gcc/testsuite/
	* gcc.dg/tree-prof/pr111559.c: New test.
---
 gcc/ipa-utils.cc                          | 17 ++++++++++-------
 gcc/testsuite/gcc.dg/tree-prof/pr111559.c | 16 ++++++++++++++++
 2 files changed, 26 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-prof/pr111559.c

diff --git a/gcc/ipa-utils.cc b/gcc/ipa-utils.cc
index 956c6294fd7..1355ccac6f0 100644
--- a/gcc/ipa-utils.cc
+++ b/gcc/ipa-utils.cc
@@ -651,13 +651,16 @@ ipa_merge_profiles (struct cgraph_node *dst,
 		{
 		  edge srce = EDGE_SUCC (srcbb, i);
 		  edge dste = EDGE_SUCC (dstbb, i);
-		  dste->probability = 
-		    dste->probability * dstbb->count.ipa ().probability_in
-						 (dstbb->count.ipa ()
-						  + srccount.ipa ())
-		    + srce->probability * srcbb->count.ipa ().probability_in
-						 (dstbb->count.ipa ()
-						  + srccount.ipa ());
+		  profile_count sum =
+		    dstbb->count.ipa () + srccount.ipa ();
+		  if (sum.nonzero_p ())
+		    dste->probability =
+		      dste->probability * dstbb->count.ipa ().probability_in
+						   (dstbb->count.ipa ()
+						    + srccount.ipa ())
+		      + srce->probability * srcbb->count.ipa ().probability_in
+						   (dstbb->count.ipa ()
+						    + srccount.ipa ());
 		}
 	      dstbb->count = dstbb->count.ipa () + srccount.ipa ();
 	    }
diff --git a/gcc/testsuite/gcc.dg/tree-prof/pr111559.c b/gcc/testsuite/gcc.dg/tree-prof/pr111559.c
new file mode 100644
index 00000000000..43202c6c888
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-prof/pr111559.c
@@ -0,0 +1,16 @@
+/* { dg-options "-O2" } */
+
+__attribute__((noipa)) static void edge(void) {}
+
+int p = 0;
+
+__attribute__((noinline))
+static void rule1(void) { if (p) edge(); }
+
+__attribute__((noinline))
+static void rule1_same(void) { if (p) edge(); }
+
+__attribute__((noipa)) int main(void) {
+    rule1();
+    rule1_same();
+}
-- 
2.42.0


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

* Re: [PATCH v2] ipa-utils: avoid uninitialized probabilities on ICF [PR111559]
  2023-10-05 12:46     ` Sergei Trofimovich
@ 2023-10-05 13:04       ` Jan Hubicka
  2023-10-05 13:11         ` [PATCH v4] " Sergei Trofimovich
  0 siblings, 1 reply; 7+ messages in thread
From: Jan Hubicka @ 2023-10-05 13:04 UTC (permalink / raw)
  To: Sergei Trofimovich; +Cc: gcc-patches, rguenther, siarheit

> diff --git a/gcc/ipa-utils.cc b/gcc/ipa-utils.cc
> index 956c6294fd7..1355ccac6f0 100644
> --- a/gcc/ipa-utils.cc
> +++ b/gcc/ipa-utils.cc
> @@ -651,13 +651,16 @@ ipa_merge_profiles (struct cgraph_node *dst,
>  		{
>  		  edge srce = EDGE_SUCC (srcbb, i);
>  		  edge dste = EDGE_SUCC (dstbb, i);
> -		  dste->probability = 
> -		    dste->probability * dstbb->count.ipa ().probability_in
> -						 (dstbb->count.ipa ()
> -						  + srccount.ipa ())
> -		    + srce->probability * srcbb->count.ipa ().probability_in
> -						 (dstbb->count.ipa ()
> -						  + srccount.ipa ());
> +		  profile_count sum =
> +		    dstbb->count.ipa () + srccount.ipa ();
> +		  if (sum.nonzero_p ())
> +		    dste->probability =
> +		      dste->probability * dstbb->count.ipa ().probability_in
> +						   (dstbb->count.ipa ()
> +						    + srccount.ipa ())
> +		      + srce->probability * srcbb->count.ipa ().probability_in
> +						   (dstbb->count.ipa ()
> +						    + srccount.ipa ());

looks good.  You can use probability_in (sum) 
in both of the places.

Honza

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

* [PATCH v4] ipa-utils: avoid uninitialized probabilities on ICF [PR111559]
  2023-10-05 13:04       ` Jan Hubicka
@ 2023-10-05 13:11         ` Sergei Trofimovich
  2023-10-05 13:27           ` Jan Hubicka
  0 siblings, 1 reply; 7+ messages in thread
From: Sergei Trofimovich @ 2023-10-05 13:11 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: gcc-patches, rguenther, siarheit

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

On Thu, Oct 05, 2023 at 03:04:55PM +0200, Jan Hubicka wrote:
> > diff --git a/gcc/ipa-utils.cc b/gcc/ipa-utils.cc
> > index 956c6294fd7..1355ccac6f0 100644
> > --- a/gcc/ipa-utils.cc
> > +++ b/gcc/ipa-utils.cc
> > @@ -651,13 +651,16 @@ ipa_merge_profiles (struct cgraph_node *dst,
> >  		{
> >  		  edge srce = EDGE_SUCC (srcbb, i);
> >  		  edge dste = EDGE_SUCC (dstbb, i);
> > -		  dste->probability = 
> > -		    dste->probability * dstbb->count.ipa ().probability_in
> > -						 (dstbb->count.ipa ()
> > -						  + srccount.ipa ())
> > -		    + srce->probability * srcbb->count.ipa ().probability_in
> > -						 (dstbb->count.ipa ()
> > -						  + srccount.ipa ());
> > +		  profile_count sum =
> > +		    dstbb->count.ipa () + srccount.ipa ();
> > +		  if (sum.nonzero_p ())
> > +		    dste->probability =
> > +		      dste->probability * dstbb->count.ipa ().probability_in
> > +						   (dstbb->count.ipa ()
> > +						    + srccount.ipa ())
> > +		      + srce->probability * srcbb->count.ipa ().probability_in
> > +						   (dstbb->count.ipa ()
> > +						    + srccount.ipa ());
> 
> looks good.  You can use probability_in (sum) 
> in both of the places.

Oh, great point! Completely forgot about it. Attached v4.

If it still looks reasonable I'll check again if `python` and
`profiledbootstrap` still survives it and will push.

-- 

  Sergei

[-- Attachment #2: v4-0001-ipa-utils-avoid-uninitialized-probabilities-on-IC.patch --]
[-- Type: text/plain, Size: 3074 bytes --]

From cb9852216b5b2524f72964b399c133557ec98df0 Mon Sep 17 00:00:00 2001
From: Sergei Trofimovich <siarheit@google.com>
Date: Wed, 27 Sep 2023 14:29:12 +0100
Subject: [PATCH v4] ipa-utils: avoid uninitialized probabilities on ICF
 [PR111559]

r14-3459-g0c78240fd7d519 "Check that passes do not forget to define profile"
exposed check failures in cases when gcc produces uninitialized profile
probabilities. In case of PR/111559 uninitialized profile is generated
by edges executed 0 times reported by IPA profile:

    $ gcc -O2 -fprofile-generate pr111559.c -o b -fopt-info
    $ ./b
    $ gcc -O2 -fprofile-use -fprofile-correction pr111559.c -o b -fopt-info

    pr111559.c: In function 'rule1':
    pr111559.c:6:13: error: probability of edge 3->4 not initialized
        6 | static void rule1(void) { if (p) edge(); }
          |             ^~~~~
    during GIMPLE pass: fixup_cfg
    pr111559.c:6:13: internal compiler error: verify_flow_info failed

The change conservatively ignores updates with zero execution counts and
uses initially assigned probabilities (`always` probability in case of
the example).

	PR ipa/111283
	PR gcov-profile/111559

gcc/
	* ipa-utils.cc (ipa_merge_profiles): Avoid producing
	uninitialized probabilities when merging counters with zero
	denominators.

gcc/testsuite/
	* gcc.dg/tree-prof/pr111559.c: New test.
---
 gcc/ipa-utils.cc                          | 15 ++++++++-------
 gcc/testsuite/gcc.dg/tree-prof/pr111559.c | 16 ++++++++++++++++
 2 files changed, 24 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-prof/pr111559.c

diff --git a/gcc/ipa-utils.cc b/gcc/ipa-utils.cc
index 956c6294fd7..6024ac69cc2 100644
--- a/gcc/ipa-utils.cc
+++ b/gcc/ipa-utils.cc
@@ -651,13 +651,14 @@ ipa_merge_profiles (struct cgraph_node *dst,
 		{
 		  edge srce = EDGE_SUCC (srcbb, i);
 		  edge dste = EDGE_SUCC (dstbb, i);
-		  dste->probability = 
-		    dste->probability * dstbb->count.ipa ().probability_in
-						 (dstbb->count.ipa ()
-						  + srccount.ipa ())
-		    + srce->probability * srcbb->count.ipa ().probability_in
-						 (dstbb->count.ipa ()
-						  + srccount.ipa ());
+		  profile_count sum =
+		    dstbb->count.ipa () + srccount.ipa ();
+		  if (sum.nonzero_p ())
+		    dste->probability =
+		      dste->probability * dstbb->count.ipa ().probability_in
+						   (sum)
+		      + srce->probability * srcbb->count.ipa ().probability_in
+						   (sum);
 		}
 	      dstbb->count = dstbb->count.ipa () + srccount.ipa ();
 	    }
diff --git a/gcc/testsuite/gcc.dg/tree-prof/pr111559.c b/gcc/testsuite/gcc.dg/tree-prof/pr111559.c
new file mode 100644
index 00000000000..43202c6c888
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-prof/pr111559.c
@@ -0,0 +1,16 @@
+/* { dg-options "-O2" } */
+
+__attribute__((noipa)) static void edge(void) {}
+
+int p = 0;
+
+__attribute__((noinline))
+static void rule1(void) { if (p) edge(); }
+
+__attribute__((noinline))
+static void rule1_same(void) { if (p) edge(); }
+
+__attribute__((noipa)) int main(void) {
+    rule1();
+    rule1_same();
+}
-- 
2.42.0


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

* Re: [PATCH v4] ipa-utils: avoid uninitialized probabilities on ICF [PR111559]
  2023-10-05 13:11         ` [PATCH v4] " Sergei Trofimovich
@ 2023-10-05 13:27           ` Jan Hubicka
  0 siblings, 0 replies; 7+ messages in thread
From: Jan Hubicka @ 2023-10-05 13:27 UTC (permalink / raw)
  To: Sergei Trofimovich; +Cc: gcc-patches, rguenther, siarheit

> On Thu, Oct 05, 2023 at 03:04:55PM +0200, Jan Hubicka wrote:
> > > diff --git a/gcc/ipa-utils.cc b/gcc/ipa-utils.cc
> > > index 956c6294fd7..1355ccac6f0 100644
> > > --- a/gcc/ipa-utils.cc
> > > +++ b/gcc/ipa-utils.cc
> > > @@ -651,13 +651,16 @@ ipa_merge_profiles (struct cgraph_node *dst,
> > >  		{
> > >  		  edge srce = EDGE_SUCC (srcbb, i);
> > >  		  edge dste = EDGE_SUCC (dstbb, i);
> > > -		  dste->probability = 
> > > -		    dste->probability * dstbb->count.ipa ().probability_in
> > > -						 (dstbb->count.ipa ()
> > > -						  + srccount.ipa ())
> > > -		    + srce->probability * srcbb->count.ipa ().probability_in
> > > -						 (dstbb->count.ipa ()
> > > -						  + srccount.ipa ());
> > > +		  profile_count sum =
> > > +		    dstbb->count.ipa () + srccount.ipa ();
> > > +		  if (sum.nonzero_p ())
> > > +		    dste->probability =
> > > +		      dste->probability * dstbb->count.ipa ().probability_in
> > > +						   (dstbb->count.ipa ()
> > > +						    + srccount.ipa ())
> > > +		      + srce->probability * srcbb->count.ipa ().probability_in
> > > +						   (dstbb->count.ipa ()
> > > +						    + srccount.ipa ());
> > 
> > looks good.  You can use probability_in (sum) 
> > in both of the places.
> 
> Oh, great point! Completely forgot about it. Attached v4.
> 
> If it still looks reasonable I'll check again if `python` and
> `profiledbootstrap` still survives it and will push.
Looks good, thanks!
Honza

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

end of thread, other threads:[~2023-10-05 13:27 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-27 14:44 [PATCH] ipa-utils: avoid generating uninitialized probabilities on merges Sergei Trofimovich
2023-10-01 19:29 ` [PATCH v2] ipa-utils: avoid uninitialized probabilities on ICF [PR111559] Sergei Trofimovich
2023-10-05 11:52   ` Jan Hubicka
2023-10-05 12:46     ` Sergei Trofimovich
2023-10-05 13:04       ` Jan Hubicka
2023-10-05 13:11         ` [PATCH v4] " Sergei Trofimovich
2023-10-05 13:27           ` Jan Hubicka

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