[LLVMdev] Assert in BlockFrequency pass

[Ivan Baev]

>
>> On 2015-Jun-04, at 12:45, Duncan P. N. Exon Smith <dexonsmith at apple.com>
>> wrote:
>>
>>> On 2015-Jun-04, at 12:28, Ivan Baev <ibaev at codeaurora.org> wrote:
>>>
>>> Hi, we got the following assert:
>>>
>>> assert(!Working[0].isLoopHeader() && "entry block is a loop header");
>>>
>>> [in BlockFrequencyInfoImpl<BT>::tryToComputeMassInFunction(),
>>> BlockFrequencyInfoImpl.h] on a Hexagon target - the entry block is a
>>> loop
>>> header. Has someone seen this assert on other targets?
>>>
>>> What would be a preferable way to fix it:
>>> - extend BlockFrequency pass to handle this case, e.g. by inserting a
>>> dummy entry block, or
>>> - make this BlockFrequncy's assumption/requirement more clearly
>>> articulated and avoid creating this situation in a preceding pass (in
>>> the
>>> particular case, a machine-level loop rotation)?
>>
>> This is unlikely to be a bug in BFI.  BFI assumes that LoopInfo is
>> correct.  This has come up before when a pass promised (but failed) to
>> preserve LoopInfo.  I think it's well enough documented -- in that BFI
>> "requires" LoopInfo -- but if you have an idea of how to make it more
>> clear that BFI requires LoopInfo go ahead :).
>>
>> Last time this came up it was suggested that we should have a better
>> way of testing/verifying that analyses have been correctly preserved,
>> and I think that would be the best way of improving the diagnostic
>> (since it would catch the problem right after machine-level loop
>> rotation fails to preserve it).
>>
>> (Of course, it's possible there *is* a bug in BFI... let me know what
>> you find.)
>>
>
> Reading the assertion, maybe there is a bug.  In LLVM IR, it's illegal
> to have a backedge to the entry block.  When I rewrote BFI I assumed
> the same was true of Machine IR.  Was I wrong?  Is it legal to have a
> backedge to the entry block?  (Or, as I assumed in my previous mail,
> did machine-level loop rotation fail to preserve LoopInfo?)
>
> If it *is* legal to have a backedge to the entry block, then we should
> update BFI to expect it.


For LLVM IR it is not legal to have a backedge to the entry block - this
is from Verifier.cpp
    // Check the entry node
    const BasicBlock *Entry = &F.getEntryBlock();
    Assert1(pred_empty(Entry),
            "Entry block to function must not have predecessors!", Entry);

Not sure about such requirement for Machine IR.

For the test case with the assert, MBB below is the entry BB.
[Loop rotate transformation in HexagonCFGOptimizer.cpp]
// We are looking for the commonly occuring patterns.
// MBB ---------
//  |           |
// (Optional    |
//  Preheader)  |
//  |           |
// MBB1 <---    |
//  |       |   |
// ...(Loop)|   |
//  |       |   |
// MBB2 ----    |
//  |           |
// MBBExit <----
//
// This pattern will be transformed into the following loop.
// i.e. the predecessor will be included in the loop.
//   ,<---------
//  |           |
// MBB ->------- -
//  |           | |
// (Optional    | |
//  Preheader)  | |
//  |           | |
// MBB1         | |
//  |           | |
// ...(Loop)    | |
//  |           | |
// MBB2 ->------  |
//  |             |
// MBBExit <------

HexagonCFGOptimizer pass does not promise to preserve LoopInfo.

Ivan