[llvm] [DomTree] Remove unnecessary domtree level check in SemiNCA (NFC) (PR #73107)

Nikita Popov via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 27 01:41:44 PST 2023


https://github.com/nikic updated https://github.com/llvm/llvm-project/pull/73107

>From 663dad2aa89e6eda88ba2e9928801220148f6cd7 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Wed, 22 Nov 2023 12:06:03 +0100
Subject: [PATCH 1/2] [DomTree] Remove unnecessary domtree level check in
 SemiNCA (NFC)

runSemiNCA() currently checks that the ReverseChildren are below
MinLevel in the DT, which is used when performing incremental
updates.

However, ReverseChildren is populated during runDFS with only
the predecessors that are part of that DFS walk, which will itself
be level limited in the relevant cases. As such, I don't believe
that this should be checked during runSemiNCA().

This code probably dates back to a time when predecessors were
run cached during runDFS and as such not limited to the visited
subtree only.
---
 .../include/llvm/Support/GenericDomTreeConstruction.h | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/llvm/include/llvm/Support/GenericDomTreeConstruction.h b/llvm/include/llvm/Support/GenericDomTreeConstruction.h
index 6c847ce54ef259a..e38f5540e17c420 100644
--- a/llvm/include/llvm/Support/GenericDomTreeConstruction.h
+++ b/llvm/include/llvm/Support/GenericDomTreeConstruction.h
@@ -268,7 +268,7 @@ struct SemiNCAInfo {
   }
 
   // This function requires DFS to be run before calling it.
-  void runSemiNCA(DomTreeT &DT, const unsigned MinLevel = 0) {
+  void runSemiNCA(DomTreeT &DT) {
     const unsigned NextDFSNum(NumToNode.size());
     SmallVector<InfoRec *, 8> NumToInfo = {nullptr};
     NumToInfo.reserve(NextDFSNum);
@@ -291,11 +291,6 @@ struct SemiNCAInfo {
         assert(NodeToInfo.contains(N) &&
                "ReverseChildren should not contain unreachable predecessors");
 
-        const TreeNodePtr TN = DT.getNode(N);
-        // Skip predecessors whose level is above the subtree we are processing.
-        if (TN && TN->getLevel() < MinLevel)
-          continue;
-
         unsigned SemiU = NumToInfo[eval(N, i + 1, EvalStack, NumToInfo)]->Semi;
         if (SemiU < WInfo.Semi) WInfo.Semi = SemiU;
       }
@@ -998,7 +993,7 @@ struct SemiNCAInfo {
     SemiNCAInfo SNCA(BUI);
     SNCA.runDFS(ToIDom, 0, DescendBelow, 0);
     LLVM_DEBUG(dbgs() << "\tRunning Semi-NCA\n");
-    SNCA.runSemiNCA(DT, Level);
+    SNCA.runSemiNCA(DT);
     SNCA.reattachExistingSubtree(DT, PrevIDomSubTree);
   }
 
@@ -1122,7 +1117,7 @@ struct SemiNCAInfo {
                       << BlockNamePrinter(PrevIDom) << "\nRunning Semi-NCA\n");
 
     // Rebuild the remaining part of affected subtree.
-    SNCA.runSemiNCA(DT, MinLevel);
+    SNCA.runSemiNCA(DT);
     SNCA.reattachExistingSubtree(DT, PrevIDom);
   }
 

>From 7ebb3bafa7c61f23785de750c7522faea28bbcbc Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Wed, 22 Nov 2023 17:04:01 +0100
Subject: [PATCH 2/2] Remove unused DT argument

---
 llvm/include/llvm/Support/GenericDomTreeConstruction.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/llvm/include/llvm/Support/GenericDomTreeConstruction.h b/llvm/include/llvm/Support/GenericDomTreeConstruction.h
index e38f5540e17c420..97529cd3277f30b 100644
--- a/llvm/include/llvm/Support/GenericDomTreeConstruction.h
+++ b/llvm/include/llvm/Support/GenericDomTreeConstruction.h
@@ -268,7 +268,7 @@ struct SemiNCAInfo {
   }
 
   // This function requires DFS to be run before calling it.
-  void runSemiNCA(DomTreeT &DT) {
+  void runSemiNCA() {
     const unsigned NextDFSNum(NumToNode.size());
     SmallVector<InfoRec *, 8> NumToInfo = {nullptr};
     NumToInfo.reserve(NextDFSNum);
@@ -571,7 +571,7 @@ struct SemiNCAInfo {
     DT.Roots = FindRoots(DT, PostViewBUI);
     SNCA.doFullDFSWalk(DT, AlwaysDescend);
 
-    SNCA.runSemiNCA(DT);
+    SNCA.runSemiNCA();
     if (BUI) {
       BUI->IsRecalculated = true;
       LLVM_DEBUG(
@@ -899,7 +899,7 @@ struct SemiNCAInfo {
 
     SemiNCAInfo SNCA(BUI);
     SNCA.runDFS(Root, 0, UnreachableDescender, 0);
-    SNCA.runSemiNCA(DT);
+    SNCA.runSemiNCA();
     SNCA.attachNewSubtree(DT, Incoming);
 
     LLVM_DEBUG(dbgs() << "After adding unreachable nodes\n");
@@ -993,7 +993,7 @@ struct SemiNCAInfo {
     SemiNCAInfo SNCA(BUI);
     SNCA.runDFS(ToIDom, 0, DescendBelow, 0);
     LLVM_DEBUG(dbgs() << "\tRunning Semi-NCA\n");
-    SNCA.runSemiNCA(DT);
+    SNCA.runSemiNCA();
     SNCA.reattachExistingSubtree(DT, PrevIDomSubTree);
   }
 
@@ -1117,7 +1117,7 @@ struct SemiNCAInfo {
                       << BlockNamePrinter(PrevIDom) << "\nRunning Semi-NCA\n");
 
     // Rebuild the remaining part of affected subtree.
-    SNCA.runSemiNCA(DT);
+    SNCA.runSemiNCA();
     SNCA.reattachExistingSubtree(DT, PrevIDom);
   }
 



More information about the llvm-commits mailing list