diff mbox

Initialize counters in autoFDO to zero, not to uninitialized.

Message ID cfc95a5c-6a5f-cd43-70dd-18c9e34700a1@suse.cz
State New
Headers show

Commit Message

Martin Liška July 11, 2017, 10:35 a.m. UTC
Hello.

This fixes majority of autoFDO test-cases.

Patch can boostrap and survives regression tests.

Ready for trunk?
Thanks,
Martin

gcc/ChangeLog:

2017-07-11  Martin Liska  <mliska@suse.cz>

	* auto-profile.c (afdo_annotate_cfg): Assign zero counts to
	BBs and edges seen by autoFDO.
---
  gcc/auto-profile.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Jeff Law July 26, 2017, 5:43 p.m. UTC | #1
On 07/11/2017 04:35 AM, Martin Liška wrote:
> Hello.
> 
> This fixes majority of autoFDO test-cases.
> 
> Patch can boostrap and survives regression tests.
> 
> Ready for trunk?
> Thanks,
> Martin
> 
> gcc/ChangeLog:
> 
> 2017-07-11  Martin Liska  <mliska@suse.cz>
> 
>     * auto-profile.c (afdo_annotate_cfg): Assign zero counts to
>     BBs and edges seen by autoFDO.
I went back and forth on this a couple times.  I could argue that if we
don't have data for the edge from auto-fdo, then the proper value is
uninitialized().  But I think the response to that argument is that if
the edge didn't show up in the afdo run, then it's counters should be
zero'd as they weren't triggered during the afdo run.

I think a comment immediately prior to the initialization seems wise. OK
with that change.

jeff
Martin Liška July 27, 2017, 12:53 p.m. UTC | #2
On 07/26/2017 07:43 PM, Jeff Law wrote:
> On 07/11/2017 04:35 AM, Martin Liška wrote:
>> Hello.
>>
>> This fixes majority of autoFDO test-cases.
>>
>> Patch can boostrap and survives regression tests.
>>
>> Ready for trunk?
>> Thanks,
>> Martin
>>
>> gcc/ChangeLog:
>>
>> 2017-07-11  Martin Liska  <mliska@suse.cz>
>>
>>     * auto-profile.c (afdo_annotate_cfg): Assign zero counts to
>>     BBs and edges seen by autoFDO.
> I went back and forth on this a couple times.  I could argue that if we
> don't have data for the edge from auto-fdo, then the proper value is
> uninitialized().  But I think the response to that argument is that if
> the edge didn't show up in the afdo run, then it's counters should be
> zero'd as they weren't triggered during the afdo run.
> 
> I think a comment immediately prior to the initialization seems wise. OK
> with that change.

Yes, I've just added comment and installed the patch as r250621.

Thanks for the review.
Martin

> 
> jeff
>
diff mbox

Patch

diff --git a/gcc/auto-profile.c b/gcc/auto-profile.c
index 71c06f30449..334f38be109 100644
--- a/gcc/auto-profile.c
+++ b/gcc/auto-profile.c
@@ -1547,9 +1547,9 @@  afdo_annotate_cfg (const stmt_set &promoted_stmts)
     edge e;
     edge_iterator ei;
 
-    bb->count = profile_count::uninitialized ();
+    bb->count = profile_count::zero ().afdo ();
     FOR_EACH_EDGE (e, ei, bb->succs)
-      e->count = profile_count::uninitialized ();
+      e->count = profile_count::zero ().afdo ();
 
     if (afdo_set_bb_count (bb, promoted_stmts))
       set_bb_annotated (bb, &annotated_bb);