Skip to content

Conversation

@vporpo
Copy link
Contributor

@vporpo vporpo commented Jan 9, 2025

This patch updates DAG's setNextNode() and setPrevNode() to update both nodes of the link.

@llvmbot
Copy link
Member

llvmbot commented Jan 9, 2025

@llvm/pr-subscribers-llvm-transforms

Author: vporpo (vporpo)

Changes

This patch updates DAG's setNextNode() and setPrevNode() to update both nodes of the link.


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

2 Files Affected:

  • (modified) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h (+12-3)
  • (modified) llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp (+3-14)
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h
index f423e1ee456cd1..00b53b42e2e572 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h
@@ -216,9 +216,18 @@ class MemDGNode final : public DGNode {
   /// Memory predecessors.
   DenseSet<MemDGNode *> MemPreds;
   friend class PredIterator; // For MemPreds.
-
-  void setNextNode(MemDGNode *N) { NextMemN = N; }
-  void setPrevNode(MemDGNode *N) { PrevMemN = N; }
+  /// Creates both edges: this<->N.
+  void setNextNode(MemDGNode *N) {
+    NextMemN = N;
+    if (NextMemN != nullptr)
+      NextMemN->PrevMemN = this;
+  }
+  /// Creates both edges: N<->this.
+  void setPrevNode(MemDGNode *N) {
+    PrevMemN = N;
+    if (PrevMemN != nullptr)
+      PrevMemN->NextMemN = this;
+  }
   friend class DependencyGraph; // For setNextNode(), setPrevNode().
   void detachFromChain() {
     if (PrevMemN != nullptr)
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp
index ba62c45a4e704e..c1a046a157d3b2 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp
@@ -283,8 +283,6 @@ void DependencyGraph::createNewNodes(const Interval<Instruction> &NewInterval) {
     // Build the Mem node chain.
     if (auto *MemN = dyn_cast<MemDGNode>(N)) {
       MemN->setPrevNode(LastMemN);
-      if (LastMemN != nullptr)
-        LastMemN->setNextNode(MemN);
       LastMemN = MemN;
     }
   }
@@ -302,7 +300,6 @@ void DependencyGraph::createNewNodes(const Interval<Instruction> &NewInterval) {
            "Wrong order!");
     if (LinkTopN != nullptr && LinkBotN != nullptr) {
       LinkTopN->setNextNode(LinkBotN);
-      LinkBotN->setPrevNode(LinkTopN);
     }
 #ifndef NDEBUG
     // TODO: Remove this once we've done enough testing.
@@ -394,22 +391,14 @@ void DependencyGraph::notifyMoveInstr(Instruction *I, const BBIterator &To) {
   if (To != BB->end()) {
     DGNode *ToN = getNodeOrNull(&*To);
     if (ToN != nullptr) {
-      MemDGNode *PrevMemN = getMemDGNodeBefore(ToN, /*IncludingN=*/false);
-      MemDGNode *NextMemN = getMemDGNodeAfter(ToN, /*IncludingN=*/true);
-      MemN->PrevMemN = PrevMemN;
-      if (PrevMemN != nullptr)
-        PrevMemN->NextMemN = MemN;
-      MemN->NextMemN = NextMemN;
-      if (NextMemN != nullptr)
-        NextMemN->PrevMemN = MemN;
+      MemN->setPrevNode(getMemDGNodeBefore(ToN, /*IncludingN=*/false));
+      MemN->setNextNode(getMemDGNodeAfter(ToN, /*IncludingN=*/true));
     }
   } else {
     // MemN becomes the last instruction in the BB.
     auto *TermN = getNodeOrNull(BB->getTerminator());
     if (TermN != nullptr) {
-      MemDGNode *PrevMemN = getMemDGNodeBefore(TermN, /*IncludingN=*/false);
-      PrevMemN->NextMemN = MemN;
-      MemN->PrevMemN = PrevMemN;
+      MemN->setPrevNode(getMemDGNodeBefore(TermN, /*IncludingN=*/false));
     } else {
       // The terminator is outside the DAG interval so do nothing.
     }

@llvmbot
Copy link
Member

llvmbot commented Jan 9, 2025

@llvm/pr-subscribers-vectorizers

Author: vporpo (vporpo)

Changes

This patch updates DAG's setNextNode() and setPrevNode() to update both nodes of the link.


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

2 Files Affected:

  • (modified) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h (+12-3)
  • (modified) llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp (+3-14)
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h
index f423e1ee456cd1..00b53b42e2e572 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h
@@ -216,9 +216,18 @@ class MemDGNode final : public DGNode {
   /// Memory predecessors.
   DenseSet<MemDGNode *> MemPreds;
   friend class PredIterator; // For MemPreds.
-
-  void setNextNode(MemDGNode *N) { NextMemN = N; }
-  void setPrevNode(MemDGNode *N) { PrevMemN = N; }
+  /// Creates both edges: this<->N.
+  void setNextNode(MemDGNode *N) {
+    NextMemN = N;
+    if (NextMemN != nullptr)
+      NextMemN->PrevMemN = this;
+  }
+  /// Creates both edges: N<->this.
+  void setPrevNode(MemDGNode *N) {
+    PrevMemN = N;
+    if (PrevMemN != nullptr)
+      PrevMemN->NextMemN = this;
+  }
   friend class DependencyGraph; // For setNextNode(), setPrevNode().
   void detachFromChain() {
     if (PrevMemN != nullptr)
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp
index ba62c45a4e704e..c1a046a157d3b2 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp
@@ -283,8 +283,6 @@ void DependencyGraph::createNewNodes(const Interval<Instruction> &NewInterval) {
     // Build the Mem node chain.
     if (auto *MemN = dyn_cast<MemDGNode>(N)) {
       MemN->setPrevNode(LastMemN);
-      if (LastMemN != nullptr)
-        LastMemN->setNextNode(MemN);
       LastMemN = MemN;
     }
   }
@@ -302,7 +300,6 @@ void DependencyGraph::createNewNodes(const Interval<Instruction> &NewInterval) {
            "Wrong order!");
     if (LinkTopN != nullptr && LinkBotN != nullptr) {
       LinkTopN->setNextNode(LinkBotN);
-      LinkBotN->setPrevNode(LinkTopN);
     }
 #ifndef NDEBUG
     // TODO: Remove this once we've done enough testing.
@@ -394,22 +391,14 @@ void DependencyGraph::notifyMoveInstr(Instruction *I, const BBIterator &To) {
   if (To != BB->end()) {
     DGNode *ToN = getNodeOrNull(&*To);
     if (ToN != nullptr) {
-      MemDGNode *PrevMemN = getMemDGNodeBefore(ToN, /*IncludingN=*/false);
-      MemDGNode *NextMemN = getMemDGNodeAfter(ToN, /*IncludingN=*/true);
-      MemN->PrevMemN = PrevMemN;
-      if (PrevMemN != nullptr)
-        PrevMemN->NextMemN = MemN;
-      MemN->NextMemN = NextMemN;
-      if (NextMemN != nullptr)
-        NextMemN->PrevMemN = MemN;
+      MemN->setPrevNode(getMemDGNodeBefore(ToN, /*IncludingN=*/false));
+      MemN->setNextNode(getMemDGNodeAfter(ToN, /*IncludingN=*/true));
     }
   } else {
     // MemN becomes the last instruction in the BB.
     auto *TermN = getNodeOrNull(BB->getTerminator());
     if (TermN != nullptr) {
-      MemDGNode *PrevMemN = getMemDGNodeBefore(TermN, /*IncludingN=*/false);
-      PrevMemN->NextMemN = MemN;
-      MemN->PrevMemN = PrevMemN;
+      MemN->setPrevNode(getMemDGNodeBefore(TermN, /*IncludingN=*/false));
     } else {
       // The terminator is outside the DAG interval so do nothing.
     }

@vporpo vporpo merged commit 9248428 into llvm:main Jan 10, 2025
11 checks passed
BaiXilin pushed a commit to BaiXilin/llvm-project that referenced this pull request Jan 12, 2025
…#122363)

This patch updates DAG's `setNextNode()` and `setPrevNode()` to update
both nodes of the link.
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.

3 participants