[llvm] [BOLT] Add sink block to flow CFG in profile inference (PR #95047)

[via llvm-commits]

================
@@ -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:

Had an offline discussion with @shawbyoung, now I see the problem (thanks for explaining this with actual CFG). So a real exit block can have exception, which makes landing pad its successor, and since landing can recover from exception and continue execution, it will go back to the function. Based on our discussions, there are a few things we need to do:

1) this issue shouldn't be unique to BOLT, worth checking how compiler is handling it. anytime we do something specific for compiler or bolt, if feels fishy because their need should be similar and it should be handled in infer without duplication.

2) we actually need to tell profi that exception flow is rare and it should take that into consideration when doing inference. one simple approximation is just to remove edges to landing pad altogether so profi won't send flow to exception path which is supposed to be very very rare anyways. doing so also solves the problem without adding sink node. However it looks like from the compiler side, we communicate such constraints via "unlikely" flag on edge, so profi can adjust edge costs. So we could also do the same for bolt. One way or another we need to profi to take it into consideration. 

It's okay if we leverage unlikely flag for EH path in a follow up patch, but we probably want an answer for #1 here. 

https://github.com/llvm/llvm-project/pull/95047