[llvm] [SandboxVec][DAG] Fix unscheduled succs when nodes are scheduled (PR #184946)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: vporpo (vporpo)

<details>
<summary>Changes</summary>

When we update use-def edges the DAG gets notified to update the UnscheduledSuccs counters. However, if either edge node is already scheduled we should not update UnscheduledSuccs because the UnscheduledSuccs counter value should be treated as "undefined" after a node has been scheduled, i.e., it's value has a meaning only before the node gets scheduled.

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


3 Files Affected:

- (modified) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h (+2-1) 
- (modified) llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp (+8-2) 
- (modified) llvm/unittests/Transforms/Vectorize/SandboxVectorizer/DependencyGraphTest.cpp (+36) 


``````````diff
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h
index 588a6eb298470..21457602c36a8 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h
@@ -98,7 +98,8 @@ class LLVM_ABI DGNode {
   // TODO: Use a PointerIntPair for SubclassID and I.
   /// For isa/dyn_cast etc.
   DGNodeID SubclassID;
-  /// The number of unscheduled successors.
+  /// The number of unscheduled successors. This is meaningless after a node
+  /// gets scheduled.
   unsigned UnscheduledSuccs = 0;
   /// This is true if this node has been scheduled.
   bool Scheduled = false;
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp
index fb7093c98cf7f..124f960a4d070 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp
@@ -537,12 +537,18 @@ void DependencyGraph::notifySetUse(const Use &U, Value *NewSrc) {
   // NewSrc if needed.
   if (auto *CurrSrcI = dyn_cast<Instruction>(U.get())) {
     if (auto *CurrSrcN = getNode(CurrSrcI)) {
-      CurrSrcN->decrUnscheduledSuccs();
+      // If CurrSrcN is scheduled there is no point in updating UnscheduleSuccs.
+      if (!CurrSrcN->scheduled()) {
+        CurrSrcN->decrUnscheduledSuccs();
+      }
     }
   }
   if (auto *NewSrcI = dyn_cast<Instruction>(NewSrc)) {
     if (auto *NewSrcN = getNode(NewSrcI)) {
-      ++NewSrcN->UnscheduledSuccs;
+      // If CurrSrcN is scheduled there is no point in updating UnscheduleSuccs.
+      if (!NewSrcN->scheduled()) {
+        ++NewSrcN->UnscheduledSuccs;
+      }
     }
   }
 }
diff --git a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/DependencyGraphTest.cpp b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/DependencyGraphTest.cpp
index 185af9544c9f7..e2514877fd0ba 100644
--- a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/DependencyGraphTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/DependencyGraphTest.cpp
@@ -1231,3 +1231,39 @@ define void @foo(i8 %v0, i8 %v1) {
   ExtUser->setOperand(0, Arg0);
   EXPECT_EQ(Add0N->getNumUnscheduledSuccs(), 1u);
 }
+
+// Don't udpate the unscheduled succs if nodes have already been scheduled.
+TEST_F(DependencyGraphTest, MaintainUnscheduledSuccsWhenScheduled) {
+  parseIR(C, R"IR(
+define void @foo(i8 %v0) {
+  %add0 = add i8 %v0, 0
+  %add1 = add i8 %v0, 1
+  %modify = add i8 %add0, 1
+  ret void
+}
+)IR");
+  llvm::Function *LLVMF = &*M->getFunction("foo");
+  sandboxir::Context Ctx(C);
+  auto *F = Ctx.createFunction(LLVMF);
+  auto *BB = &*F->begin();
+  auto It = BB->begin();
+  auto *Add0 = cast<sandboxir::BinaryOperator>(&*It++);
+  auto *Add1 = cast<sandboxir::BinaryOperator>(&*It++);
+  auto *Modify = cast<sandboxir::BinaryOperator>(&*It++);
+  // DAG contains all Add0, Add1 and Modify
+  sandboxir::DependencyGraph DAG(getAA(*LLVMF), Ctx);
+  DAG.extend({Add0, Add1, Modify});
+  auto *Add0N = DAG.getNode(Add0);
+  auto *Add1N = DAG.getNode(Add1);
+  // Mark Add0N and Add1N as scheduled.
+  Add0N->setScheduled(true);
+  Add1N->setScheduled(true);
+  EXPECT_EQ(Add0N->getNumUnscheduledSuccs(), 1u);
+  EXPECT_EQ(Add1N->getNumUnscheduledSuccs(), 0u);
+
+  // Change Modify's operand and make sure we won't update Add0N's or Add1N's
+  // unscheduled succs.
+  Modify->setOperand(0, Add1);
+  EXPECT_EQ(Add0N->getNumUnscheduledSuccs(), 1u);
+  EXPECT_EQ(Add1N->getNumUnscheduledSuccs(), 0u);
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/184946