Re: [PATCH] Sanitize block partitioning under -freorder-blocks-and-partition

[Teresa Johnson <tejohnson at google dot com>]

On Fri, Aug 2, 2013 at 4:04 PM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> On Fri, Aug 2, 2013 at 5:05 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>> +/* Called when block BB has been reassigned to a different partition,
>>> +   to ensure that the region crossing attributes are updated.  */
>>> +
>>> +static void
>>> +fixup_bb_partition (basic_block bb)
>>> +{
>>> +  edge e;
>>> +  edge_iterator ei;
>>> +
>>> +  /* Now need to make bb's pred edges non-region crossing.  */
>>> +  FOR_EACH_EDGE (e, ei, bb->preds)
>>> +    {
>>> +      fixup_partition_crossing (e);
>>> +    }
>>> +
>>> +  /* Possibly need to make bb's successor edges region crossing,
>>> +     or remove stale region crossing.  */
>>> +  FOR_EACH_EDGE (e, ei, bb->succs)
>>> +    {
>>> +      if ((e->flags & EDGE_FALLTHRU)
>>> +          && BB_PARTITION (bb) != BB_PARTITION (e->dest)
>>> +          && e->dest != EXIT_BLOCK_PTR)
>>> +        force_nonfallthru (e);
>>> +      else
>>> +        fixup_partition_crossing (e);
>>> +    }
>>> +}
>>
>> Is there particular reason why preds can not be fallhtrus
>
> Yes, by definition a crossing edge cannot fall through. There is
> always a control transfer from one section to another.
>
>
>>> +/* Sanity check partition hotness to ensure that basic blocks in
>>> +   the cold partition don't dominate basic blocks in the hot partition.
>>> +   If FLAG_ONLY is true, report violations as errors. Otherwise
>>> +   re-mark the dominated blocks as cold, since this is run after
>>> +   cfg optimizations that may make hot blocks previously reached
>>> +   by both hot and cold blocks now only reachable along cold paths.  */
>>
>> With profile, I suppose we can have cold blocks dominating hot blocks when the
>> hot blocks is in loop whose trip count is high enough.
>
> That is the common case, actually.
>
>>  Indeed for partitioning
>> reasons it does not make sense to push those into different section.
>
> The partitioning algrorithm makes sure this doesn't happen. The
> hottest path from the entry block to a hot basic block is always part
> of the hot partition.

Well, at least with this patch that will be true. The trunk version
just partitions based on the bb's count without regard to paths.

Thanks,
Teresa

>
>
>> I also wonder, if we finally get the pass stable, can we enable it by default
>> and offline probably cold blocks w/o profile?
>
> That is the general idea behind all this work, obviously ;-)
>
>> Primarily blocks reachable only
>> by EH + blocks leading to a crash or throw().  For C++ those should be common
>> enough to make a difference...
>
> Yup, and IIRC Theresa posted some numbers that showed this.
>
> Ciao!
> Steven



-- 
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413