Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DomTree] Store ReverseChildren as indices (NFC) #73505

Merged
merged 1 commit into from
Nov 28, 2023
Merged

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Nov 27, 2023

Store the ReverseChildren using node indices instead of node pointers. This avoids some more hash table lookups.

I've also increased the size of the SmallVector from 2 to 4. As the indices are half as large as the pointers (on 64bit) this keeps memory usage the same as before. I've found the larger SmallVector to perform a bit better.

Compile-time results: http://llvm-compile-time-tracker.com/compare.php?from=272085f10bfaa33349682a9e4d08c990c159355d&to=3df05125000536a0c954dacac2d5a1b232be3efe&stat=instructions%3Au

Store the ReverseChildren using node indices instead of node
pointers. This avoids some more hash table lookups.

I've also increased the size of the SmallVector from 2 to 4. As
the indices are half as large as the pointers (on 64bit) this
keeps memory usage the same as before. I've found the larger
SmallVector to perform a bit better.
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 27, 2023

@llvm/pr-subscribers-llvm-support

Author: Nikita Popov (nikic)

Changes

Store the ReverseChildren using node indices instead of node pointers. This avoids some more hash table lookups.

I've also increased the size of the SmallVector from 2 to 4. As the indices are half as large as the pointers (on 64bit) this keeps memory usage the same as before. I've found the larger SmallVector to perform a bit better.

Compile-time results: http://llvm-compile-time-tracker.com/compare.php?from=272085f10bfaa33349682a9e4d08c990c159355d&to=3df05125000536a0c954dacac2d5a1b232be3efe&stat=instructions%3Au


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

1 Files Affected:

  • (modified) llvm/include/llvm/Support/GenericDomTreeConstruction.h (+6-9)
diff --git a/llvm/include/llvm/Support/GenericDomTreeConstruction.h b/llvm/include/llvm/Support/GenericDomTreeConstruction.h
index 97529cd3277f30b..4ecf492b2847489 100644
--- a/llvm/include/llvm/Support/GenericDomTreeConstruction.h
+++ b/llvm/include/llvm/Support/GenericDomTreeConstruction.h
@@ -67,7 +67,7 @@ struct SemiNCAInfo {
     unsigned Semi = 0;
     unsigned Label = 0;
     NodePtr IDom = nullptr;
-    SmallVector<NodePtr, 2> ReverseChildren;
+    SmallVector<unsigned, 4> ReverseChildren;
   };
 
   // Number to node mapping is 1-based. Initialize the mapping to start with
@@ -205,7 +205,7 @@ struct SemiNCAInfo {
         // Don't visit nodes more than once but remember to collect
         // ReverseChildren.
         if (SIT != NodeToInfo.end() && SIT->second.DFSNum != 0) {
-          if (Succ != BB) SIT->second.ReverseChildren.push_back(BB);
+          if (Succ != BB) SIT->second.ReverseChildren.push_back(LastNum);
           continue;
         }
 
@@ -216,7 +216,7 @@ struct SemiNCAInfo {
         auto &SuccInfo = NodeToInfo[Succ];
         WorkList.push_back(Succ);
         SuccInfo.Parent = LastNum;
-        SuccInfo.ReverseChildren.push_back(BB);
+        SuccInfo.ReverseChildren.push_back(LastNum);
       }
     }
 
@@ -236,10 +236,10 @@ struct SemiNCAInfo {
   //
   // For each vertex V, its Label points to the vertex with the minimal sdom(U)
   // (Semi) in its path from V (included) to NodeToInfo[V].Parent (excluded).
-  unsigned eval(NodePtr V, unsigned LastLinked,
+  unsigned eval(unsigned V, unsigned LastLinked,
                 SmallVectorImpl<InfoRec *> &Stack,
                 ArrayRef<InfoRec *> NumToInfo) {
-    InfoRec *VInfo = &NodeToInfo[V];
+    InfoRec *VInfo = NumToInfo[V];
     if (VInfo->Parent < LastLinked)
       return VInfo->Label;
 
@@ -287,10 +287,7 @@ struct SemiNCAInfo {
 
       // Initialize the semi dominator to point to the parent node.
       WInfo.Semi = WInfo.Parent;
-      for (const auto &N : WInfo.ReverseChildren) {
-        assert(NodeToInfo.contains(N) &&
-               "ReverseChildren should not contain unreachable predecessors");
-
+      for (unsigned N : WInfo.ReverseChildren) {
         unsigned SemiU = NumToInfo[eval(N, i + 1, EvalStack, NumToInfo)]->Semi;
         if (SemiU < WInfo.Semi) WInfo.Semi = SemiU;
       }

Copy link

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff f21a70f9fe21539f70212ba2346c3db54f4d9980 5bc1fb1f188d55e9549cf3f52628e11c4faa1d1f -- llvm/include/llvm/Support/GenericDomTreeConstruction.h
View the diff from clang-format here.
diff --git a/llvm/include/llvm/Support/GenericDomTreeConstruction.h b/llvm/include/llvm/Support/GenericDomTreeConstruction.h
index 4ecf492b28..795c84825f 100644
--- a/llvm/include/llvm/Support/GenericDomTreeConstruction.h
+++ b/llvm/include/llvm/Support/GenericDomTreeConstruction.h
@@ -205,7 +205,8 @@ struct SemiNCAInfo {
         // Don't visit nodes more than once but remember to collect
         // ReverseChildren.
         if (SIT != NodeToInfo.end() && SIT->second.DFSNum != 0) {
-          if (Succ != BB) SIT->second.ReverseChildren.push_back(LastNum);
+          if (Succ != BB)
+            SIT->second.ReverseChildren.push_back(LastNum);
           continue;
         }
 

Copy link
Member

@kuhar kuhar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nikic nikic merged commit aefca74 into llvm:main Nov 28, 2023
2 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants