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)? Thanks, Ivan
> 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.)
> 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.