[llvm] [BOLT] Add sink block to flow CFG in profile inference (PR #95047)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 13 10:51:52 PDT 2024
================
@@ -309,23 +309,28 @@ createFlowFunction(const BinaryFunction::BasicBlockOrderType &BlockOrder) {
FlowFunction Func;
// Add a special "dummy" source so that there is always a unique entry point.
- // Because of the extra source, for all other blocks in FlowFunction it holds
- // that Block.Index == BB->getIndex() + 1
FlowBlock EntryBlock;
EntryBlock.Index = 0;
Func.Blocks.push_back(EntryBlock);
- // Create FlowBlock for every basic block in the binary function
+ // Create FlowBlock for every basic block in the binary function.
for (const BinaryBasicBlock *BB : BlockOrder) {
Func.Blocks.emplace_back();
FlowBlock &Block = Func.Blocks.back();
Block.Index = Func.Blocks.size() - 1;
+ Block.IsExit = BB->successors().empty();
(void)BB;
assert(Block.Index == BB->getIndex() + 1 &&
"incorrectly assigned basic block index");
}
- // Create FlowJump for each jump between basic blocks in the binary function
+ // Add a special "dummy" sink block so there is always a unique sink.
----------------
WenleiHe wrote:
> "block has no successors" does not fully encompass our definition of "exit blocks"
What is our definition of "exit blocks"? A reasonable definition could be a block that transfer control outside of the function. Now successor blocks naturally implies the successor within the same function. With that definition, blocks containing no return calls should not have successors, and such information should be embeded in the CFG already (`isExitBlock`), without needing to look at BinaryBasicBlock again. If we do need extra info to know where we have exit flow due to noreturn call after CFG is constructed, that indicates the CFG construction is wrong?
> I plan to cover these degenerate cases in a following commit.
Evidently, the way this changes are split causes confusion. This patch as it is doesn't make its intention obvious. It would be better to just include those intended changes in one patch.
https://github.com/llvm/llvm-project/pull/95047
More information about the llvm-commits
mailing list