[v4] ipa-utils: avoid uninitialized probabilities on ICF [PR111559]

Message ID ZR62FFoMl3B8i88F@nz
State Accepted
Headers
Series [v4] ipa-utils: avoid uninitialized probabilities on ICF [PR111559] |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Sergei Trofimovich Oct. 5, 2023, 1:11 p.m. UTC
  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.
  

Comments

Jan Hubicka Oct. 5, 2023, 1:27 p.m. UTC | #1
> 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
  

Patch

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();
+}