[PATCH] D156903: [FuncSpec] Estimate dead blocks more accurately.

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu accepted this revision.
ChuanqiXu added a comment.
This revision is now accepted and ready to land.

LGTM with nit.

Sorry that I imaged I commented on this : (



================
Comment at: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp:109-117
+static bool canEliminateSuccessor(BasicBlock *DeadBB, BasicBlock *SuccBB,
+                                  DenseSet<BasicBlock *> &DeadBlocks) {
+  unsigned I = 0;
+  return all_of(predecessors(SuccBB),
+    [&I, DeadBB, SuccBB, &DeadBlocks] (BasicBlock *PredBB) {
+    return I++ < MaxBlockPredecessors &&
+      (PredBB == DeadBB || PredBB == SuccBB || DeadBlocks.contains(PredBB));
----------------
labrinea wrote:
> ChuanqiXu wrote:
> > My instinct reaction to the function is "doesn't DeadBlocks contain DeadBB"?
> Not yet. If isBlockExecutable and canEliminateSuccessor, then we push it to the work list. Blocks are marked dead after being popped from the work list.
Got it.

nit: Then I feel slightly better to make it a static private member function of `InstCostVisitor`. Then the reader may be less confused.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156903