[PATCH] D31701: [BPI] Refactor post domination calculation and simple fix for ColdCall

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added inline comments.


================
Comment at: lib/Analysis/BranchProbabilityInfo.cpp:150-156
+  bool MarkColdCall = true;
+  for (auto *I : successors(BB))
+    // If any of successor is not post dominated then BB is also not.
+    if (!PostDominatedByColdCall.count(I)) {
+      MarkColdCall = false;
+      break;
+    }
----------------
You can rewrite this without the loop:

  if (llvm::all_of(successors(BB), [&](BasicBlock *SuccBB) {
        return PostDominatedByColdCall.count(SuccBB);
      })) {
    PostDominatedByColdCall.insert(BB);
    return;
  }


================
Comment at: lib/Analysis/BranchProbabilityInfo.cpp:161-166
+    for (BasicBlock::const_iterator I = BB->begin(), E = BB->end(); I != E; ++I)
+      if (const CallInst *CI = dyn_cast<CallInst>(I))
+        if (CI->hasFnAttr(Attribute::Cold)) {
+          MarkColdCall = true;
+          break;
+        }
----------------
Use a range based for loop over the instructions, and directly insert and return here?


================
Comment at: lib/Analysis/BranchProbabilityInfo.cpp:170-174
+    // If the terminator is an InvokeInst, check only the normal destination
+    // block as the unwind edge of InvokeInst is also very unlikely taken.
+    if (auto *II = dyn_cast<InvokeInst>(TI))
+      if (PostDominatedByColdCall.count(II->getNormalDest()))
+        MarkColdCall = true;
----------------
Do this first (before walking successors)? Then you can just insert and return rather than needing a bool.


https://reviews.llvm.org/D31701