Skip to content

Conversation

@vporpo
Copy link
Contributor

@vporpo vporpo commented Jan 24, 2025

This patch fixes the scheduler's clear() function to also clear the ReadyList. Not doing so is a bug and results in crashes when the ReadyList contains stale instructions, because it was never clered.

This patch fixes the scheduler's clear() function to also clear
the ReadyList. Not doing so is a bug and results in crashes when the
ReadyList contains stale instructions, because it was never clered.
@llvmbot
Copy link
Member

llvmbot commented Jan 24, 2025

@llvm/pr-subscribers-vectorizers

@llvm/pr-subscribers-llvm-transforms

Author: vporpo (vporpo)

Changes

This patch fixes the scheduler's clear() function to also clear the ReadyList. Not doing so is a bug and results in crashes when the ReadyList contains stale instructions, because it was never clered.


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

2 Files Affected:

  • (modified) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h (+7)
  • (modified) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Scheduler.h (+3)
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h
index 00b53b42e2e572..b2d7c9b8aa8bbc 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h
@@ -417,6 +417,13 @@ class DependencyGraph {
     DAGInterval = {};
   }
 #ifndef NDEBUG
+  /// \Returns true if the DAG's state is clear. Used in assertions.
+  bool empty() const {
+    bool IsEmpty = InstrToNodeMap.empty();
+    assert(IsEmpty == DAGInterval.empty() &&
+           "Interval and InstrToNodeMap out of sync!");
+    return IsEmpty;
+  }
   void print(raw_ostream &OS) const;
   LLVM_DUMP_METHOD void dump() const;
 #endif // NDEBUG
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Scheduler.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Scheduler.h
index 52891c3f7535c9..25432e1396c73f 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Scheduler.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Scheduler.h
@@ -164,7 +164,10 @@ class Scheduler {
     Bndls.clear();
     // TODO: clear view once it lands.
     DAG.clear();
+    ReadyList.clear();
     ScheduleTopItOpt = std::nullopt;
+    assert(Bndls.empty() && DAG.empty() && ReadyList.empty() &&
+           !ScheduleTopItOpt && "Expected empty state!");
   }
 
 #ifndef NDEBUG

@vporpo vporpo merged commit 6db73fa into llvm:main Jan 24, 2025
11 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.

3 participants