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

2023-10-05 Thread Jan Hubicka
> 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 v4] ipa-utils: avoid uninitialized probabilities on ICF [PR111559]

2023-10-05 Thread Sergei Trofimovich
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
>From cb9852216b5b2524f72964b399c133557ec98df0 Mon Sep 17 00:00:00 2001
From: Sergei Trofimovich 
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 ();