[LLVMdev] Assert in BlockFrequency pass

[Duncan P. N. Exon Smith]

> On 2015-Jun-05, at 19:31, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> 
> 
>> On 2015 Jun 5, at 18:08, Ivan Baev <ibaev at codeaurora.org> wrote:
>> 
>>> 
>>>> 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.
> 
> I can't find any mention of my assumption in the machine IR
> documentation; it looks like I was just wrong :/.
> 
> This shouldn't be hard to fix -- I just need to massage some logic
> here and there -- but I'll need your help coming up with a testcase.
> Please file a PR with the testcase attached and CC me, and I'll fix it
> as soon as I can!

Ping?  (Did I miss the PR?)

> 
>> 
>> 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
>> 
>> 
> 
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev