Re: if-conversion/HOT-COLD partitioning

[Christophe Lyon <christophe dot lyon at linaro dot org>]

On 24 October 2012 22:07, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> On Wed, Oct 24, 2012 at 6:11 PM, Christophe Lyon wrote:
>> On 24 October 2012 00:42, Steven Bosscher wrote:
>>> On Tue, Oct 23, 2012 at 10:29 PM, Christophe Lyon wrote:
>>>> Well, both of these functions appear to check that the 2 blocks to
>>>> merge belong to the same partition, so it should be OK.
>>>
>>> In your first email, you said if-convert was merging two blocks from
>>> different partitions. can_merge_block_p() would rejected merging the
>>> two blocks, so merge_blocks shouldn't be called on them.
>>>
>>> IIRC cfghooks.c:merge_blocks() used to have a
>>> gcc_assert(can_merge_blocks(a,b)) but it's not there now. But if
>>> can_merge_blocks() returns false, merge_blocks should fail. Your bug
>>> is that merge_blocks is being called at all on those blocks from
>>> different partitions.
>>>
>>>
>>>> But not all calls to merge_blocks are guarded by if
>>>> (can_merge_block_p()), this is probably where the problem is?
>>>
>>> Not sure. Depends on what blocks get merged. It may be that
>>> if-conversion shouldn't even be attempting whatever transformation
>>> it's attempting. Not enough information.
>>>
>>
>> What happens is that merge_if_block() is called with test_bb, then_bb
>> and else_bb in the cold section, while join_bb is in the hot one.
>
> AFAICT that can only happen if the join_bb has more predecessors than
> just then_bb and else_bb. Otherwise, you'd be looking at a complete
> diamond region, and test_bb and either else_bb or then_bb should be in
> the hot partition as well. But if the join_bb has more predecessors,
> then merge_blocks shouldn't be able to merge away the join block.
>
> So either something's wrong with the CFG so that merge_if_blocks sees
> a join_bb with fewer than 2 predecessors (the only path to the
> merge_blocks call in merge_if_blocks), or the profile data is so
> corrupted that the partitioning has somehow gone wrong. So...
>
It looks like something is wrong with the CFG:

       |
   19 (COLD)
    /     \
   /       \
20 (COLD)  21 (COLD)
   \       /
    \     /
     22 (HOT)

but indeed we have EDGE_COUNT (join_bb->preds) == 1


>> merge_if_block calls merge_blocks unconditionally several times (in my
>> case, the wrong one is merge_blocks (combo_bb, join_bb)).
>
> ... still not quite enough information.
>
> A more detailed explanation of the paths through the code that lead to
> the error would be nice. A test case would be good. A PR would be
> best.

I understand; the problem is that I am not allowed to publish the
input code leading to this situation :-(

Thanks for your help,

Christophe.