> Hi Jan,
>
> I didn't find
>
> https://gcc.gnu.org/cgit/gcc/commit/?id=8498ef3d075801
>
> in
>
> https://gcc.gnu.org/pipermail/gcc-patches/2025-October/date.html
>
> In any case, your commit caused:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=122122
Hello,
I apologize for this. The patch in question was the following. I am
not sure why it did not read mailing list.
Improve profile update in merge_blocks
When merging blocks we currently alway use count of the first basic block.
In some cases we merge block containing call to cold noreturn function (thus
having count 0 (reliable)) with earlier block with weaker form of profile.
In this case we can still preserve reliable count of 0.
The patch also makes block merging to pick higher of the counts if quality
is the same. This should reduce chances of losing track of hot code in broken
profiles.
Bootstrapped/regtested x86_64-linux, comitted.
gcc/ChangeLog:
* cfghooks.cc (merge_blocks): Choose more reliable or higher BB
count.
diff --git a/gcc/cfghooks.cc b/gcc/cfghooks.cc
index 5f7fc272374..8b3346898aa 100644
--- a/gcc/cfghooks.cc
+++ b/gcc/cfghooks.cc
@@ -817,6 +817,15 @@ merge_blocks (basic_block a, basic_block b)
if (!cfg_hooks->merge_blocks)
internal_error ("%s does not support merge_blocks", cfg_hooks->name);
+ /* Pick the more reliable count. If both qualities agrees, pick the larger
+ one since turning mistakely hot code to cold is more harmful. */
+ if (a->count.initialized_p ())
+ a->count = b->count;
+ else if (a->count.quality () < b->count.quality ())
+ a->count = b->count;
+ else if (a->count.quality () == b->count.quality ())
+ a->count = a->count.max (b->count);
+
cfg_hooks->merge_blocks (a, b);
if (current_loops != NULL)
Now the bug is reversed first conditional about unitialized. If
a->count is unitialized while b->count is, we are better to use it.
While I had rest of the patch in my tree for a while, I added this test
late and missing !. Thanks for spotting it.
I see that your patch also tests for block b being non-empty.
I am not sure this is a good idea. Even empty block can hold meaningful
profile. We can experiment with that separately.
I am testing the obvious fix and will commit it
Fix handling of uninitialized counts in merge_blocks
gcc/ChangeLog:
* cfghooks.cc (merge_blocks): Fix typo in the previous change.
Co-authored-by: H.J. Lu <[email protected]>
diff --git a/gcc/cfghooks.cc b/gcc/cfghooks.cc
index 8b3346898aa..25bc5d4b273 100644
--- a/gcc/cfghooks.cc
+++ b/gcc/cfghooks.cc
@@ -819,7 +819,7 @@ merge_blocks (basic_block a, basic_block b)
/* Pick the more reliable count. If both qualities agrees, pick the larger
one since turning mistakely hot code to cold is more harmful. */
- if (a->count.initialized_p ())
+ if (!a->count.initialized_p ())
a->count = b->count;
else if (a->count.quality () < b->count.quality ())
a->count = b->count;
