The patch looks OK to me in general (I can not approve it).
Still have one question...
> +
> +/* Ensure that no cold bbs dominate hot bbs along the dominance or
> +   post-dominance DIR, for example as a result of edge weight insanities.
> +   Returns the updated value of COLD_BB_COUNT and adds newly-hot bbs
> +   to BBS_IN_HOT_PARTITION.  */
> +
> +static unsigned int
> +sanitize_dominator_hotness (enum cdi_direction dir, unsigned int 
> cold_bb_count,
> +                            vec<basic_block> *bbs_in_hot_partition)
> +{
> +  /* Callers check this.  */
> +  gcc_checking_assert (cold_bb_count);
> +
> +  bool dom_calculated_here = !dom_info_available_p (dir);
> +
> +  if (dom_calculated_here)
> +    calculate_dominance_info (dir);
> +
> +  /* Keep examining hot bbs while we still have some left to check
> +     and there are remaining cold bbs.  */
> +  vec<basic_block> hot_bbs_to_check = bbs_in_hot_partition->copy ();
> +  while (! hot_bbs_to_check.is_empty ()
> +         && cold_bb_count)
> +    {
> +      basic_block bb = hot_bbs_to_check.pop ();
> +      basic_block dom_bb = get_immediate_dominator (dir, bb);
> +
> +      /* If bb's immediate dominator is also hot (or unpartitioned,
> +         e.g. the entry block) then it is ok. If it is cold, it
> +         needs to be adjusted.  */
> +      if (BB_PARTITION (dom_bb) != BB_COLD_PARTITION)
> +        continue;

What will hapepn on

if (t)
  something
else
  something else
for (i=0;i<1000000;i++)
  something else2

I would expect if/something and something else to be cold by profile feedback.
Your dominator code will bring the if into hot partition but both paths
of conditional will be cold, so the number of crossings will actually grow.

If we want to have at least some path to hot blocks in the hot region, I suspect
we could walk back from hot regions to entry and keep those in hot regions 
rather
than relying on the dominator tree...
But I am sure such things can be dealt with incrementally.

Honza

Reply via email to