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

[Alexandros Lamprineas via Phabricator via llvm-commits]

labrinea added a comment.

So generally, when we encounter root dead blocks, then their predecessor will not be in DeadBlocks. Successors of known dead blocks do have at least one predecessor in DeadBlocks (the one we followed while traversing the graph).



================
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));
----------------
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.


================
Comment at: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp:158
+      if (isBlockExecutable(SuccBB) &&
+          canEliminateSuccessor(BB, SuccBB, DeadBlocks))
         WorkList.push_back(SuccBB);
----------------
Here, BB, which is the predecessor of SuccBB, is indeed already in DeadBlocks.


================
Comment at: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp:247
+    if (BB != Succ && isBlockExecutable(BB) &&
+        canEliminateSuccessor(I.getParent(), BB, DeadBlocks))
+      WorkList.push_back(BB);
----------------
Here, BB corresponds to the not taken branch. If I.getParent is the predecessor of BB (despite not being in DeadBlocks), we can eliminate BB.


================
Comment at: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp:265
+  if (isBlockExecutable(Succ) &&
+      canEliminateSuccessor(I.getParent(), Succ, DeadBlocks))
     WorkList.push_back(Succ);
----------------
Here, Succ corresponds to the not taken branch. If I.getParent is the predecessor of Succ (despite not being in DeadBlocks), we can eliminate Succ.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156903