[PredicateInfo] Don't use depth first walk (NFCI)

[nikic]

The order in which we collect the predicates does not matter, as they will be sorted anyway. As such, avoid the expensive depth first walk over the dominator tree and instead use plain iteration over the function.

(To be a bit more precise, the predicates and uses for a specific value are sorted, so this change has no impact on that. It can change the order in which values are processed in the first place, but that order is not semantically relevant.)

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Nikita Popov (nikic)

Changes

The order in which we collect the predicates does not matter, as they will be sorted anyway. As such, avoid the expensive depth first walk over the dominator tree and instead use plain iteration over the function.

(To be a bit more precise, the predicates and uses for a specific value are sorted, so this change has no impact on that. It can change the order in which values are processed in the first place, but that order is not semantically relevant.)

---

Full diff: https://github.com/llvm/llvm-project/pull/145016.diff

1 Files Affected:

• (modified) llvm/lib/Transforms/Utils/PredicateInfo.cpp (+8-6)

diff --git a/llvm/lib/Transforms/Utils/PredicateInfo.cpp b/llvm/lib/Transforms/Utils/PredicateInfo.cpp
index 9b239d9161e7f..67defef3f75f9 100644
--- a/llvm/lib/Transforms/Utils/PredicateInfo.cpp
+++ b/llvm/lib/Transforms/Utils/PredicateInfo.cpp
@@ -488,17 +488,19 @@ void PredicateInfoBuilder::buildPredicateInfo() {
   // Collect operands to rename from all conditional branch terminators, as well
   // as assume statements.
   SmallVector<Value *, 8> OpsToRename;
-  for (auto *DTN : depth_first(DT.getRootNode())) {
-    BasicBlock *BranchBB = DTN->getBlock();
-    if (auto *BI = dyn_cast<BranchInst>(BranchBB->getTerminator())) {
+  for (BasicBlock &BB : F) {
+    if (!DT.isReachableFromEntry(&BB))
+      continue;
+
+    if (auto *BI = dyn_cast<BranchInst>(BB.getTerminator())) {
       if (!BI->isConditional())
         continue;
       // Can't insert conditional information if they all go to the same place.
       if (BI->getSuccessor(0) == BI->getSuccessor(1))
         continue;
-      processBranch(BI, BranchBB, OpsToRename);
-    } else if (auto *SI = dyn_cast<SwitchInst>(BranchBB->getTerminator())) {
-      processSwitch(SI, BranchBB, OpsToRename);
+      processBranch(BI, &BB, OpsToRename);
+    } else if (auto *SI = dyn_cast<SwitchInst>(BB.getTerminator())) {
+      processSwitch(SI, &BB, OpsToRename);
     }
   }
   for (auto &Assume : AC.assumptions()) {

[dtcxzyw]

https://llvm-compile-time-tracker.com/compare.php?from=d5e03358aacaf50d5f99d78273acdbd59b00dac1&to=98fe4b5dcac18abf91e4ea58b0f0f877ab75773f&stat=instructions:u

[dtcxzyw]

IIRC the orders in OpsToRename and OperandInfo.Infos don't matter, as we will perform a stable sort later on OrderedUses, right?

[nikic]

IIRC the orders in OpsToRename and OperandInfo.Infos don't matter, as we will perform a stable sort later on OrderedUses, right?

They don't matter in the sense that it's not important for correctness. I believe the order of the inserted ssa.copy instructions can change as a result, but the instructions themselves will be the same.

[dtcxzyw]

LG