[PATCH] D89088: [MBP] Add whole chain to BlockFilterSet instead of individual BB

[Guozhi Wei via Phabricator via llvm-commits]

Carrot added a comment.

@dmajor, without a test case it is difficult to point out what's wrong since there is no obvious relation between this patch and VarLocBasedImpl.cpp. I will try my best to do some simple analysis/guess:

VarLocBasedImpl.cpp runs a dataflow algorithm on a function to collect debug information. 
This patch only impacts the field UnscheduledPredecessors of a BlockChain, it is mostly used as a heuristic in block layout algorithms. This patch doesn't directly modify control flow. So it triggered some unknown bug in either MachineBlockPlacement or VarLocBasedImpl.

Let's look at the assertion at VarLocBasedImpl.cpp:1668, the comment says the MBB is processed in reverse post-order, it expects either one of its predecessor is processed or MBB is an entry block. Assume the algorithm is correctly implemented, then the only situation which can trigger this assert is MBB doesn't have a predecessor and is not entry block. But in function VarLocBasedLDV::ExtendRanges, the MBB passed to join() is extracted from Worklist, which is the result of ReversePostOrderTraversal, so MBB must have a predecessor, otherwise it can't be put into Worklist. So the only explanation is there is something wrong in the algorithm implementation.
Another possibility is either MBB.preds or MBB.succs are not correctly maintained although it is not very likely.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89088/new/

https://reviews.llvm.org/D89088