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

[SandboxVec][Interval] Implement Interval::comesBefore() #112026

Merged
merged 1 commit into from
Oct 11, 2024

Conversation

vporpo
Copy link
Contributor

@vporpo vporpo commented Oct 11, 2024

This patch implements Interval::comesBefore(const Interval &Other) which returns true if this interval is strictly before Other in program order. The function asserts that the intervals are disjoint.

This patch implements `Interval::comesBefore(const Interval &Other)` which
returns true if this interval is strictly before Other in program order.
The function asserts that the intervals are disjoint.
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 11, 2024

@llvm/pr-subscribers-vectorizers

@llvm/pr-subscribers-llvm-transforms

Author: vporpo (vporpo)

Changes

This patch implements Interval::comesBefore(const Interval &Other) which returns true if this interval is strictly before Other in program order. The function asserts that the intervals are disjoint.


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

3 Files Affected:

  • (modified) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Interval.h (+6)
  • (modified) llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp (+1-5)
  • (modified) llvm/unittests/Transforms/Vectorize/SandboxVectorizer/IntervalTest.cpp (+19)
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Interval.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Interval.h
index e0c581f1d50b40..0777d9b043eb62 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Interval.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Interval.h
@@ -127,6 +127,12 @@ template <typename T> class Interval {
   }
   /// Inequality.
   bool operator!=(const Interval &Other) const { return !(*this == Other); }
+  /// \Returns true if this interval comes before \p Other in program order.
+  /// This expects disjoint intervals.
+  bool comesBefore(const Interval &Other) const {
+    assert(disjoint(Other) && "Expect disjoint intervals!");
+    return bottom()->comesBefore(Other.top());
+  }
   /// \Returns true if this and \p Other have nothing in common.
   bool disjoint(const Interval &Other) const {
     if (Other.empty())
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp
index db58069de47051..a8e362571837cc 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp
@@ -231,11 +231,7 @@ void DependencyGraph::createNewNodes(const Interval<Instruction> &NewInterval) {
   }
   // Link new MemDGNode chain with the old one, if any.
   if (!DAGInterval.empty()) {
-    // TODO: Implement Interval::comesBefore() to replace this check.
-    bool NewIsAbove = NewInterval.bottom()->comesBefore(DAGInterval.top());
-    assert(
-        (NewIsAbove || DAGInterval.bottom()->comesBefore(NewInterval.top())) &&
-        "Expected NewInterval below DAGInterval.");
+    bool NewIsAbove = NewInterval.comesBefore(DAGInterval);
     const auto &TopInterval = NewIsAbove ? NewInterval : DAGInterval;
     const auto &BotInterval = NewIsAbove ? DAGInterval : NewInterval;
     MemDGNode *LinkTopN =
diff --git a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/IntervalTest.cpp b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/IntervalTest.cpp
index ee461e48f0dc03..b04e4fc7cffcae 100644
--- a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/IntervalTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/IntervalTest.cpp
@@ -123,6 +123,25 @@ define void @foo(i8 %v0) {
     EXPECT_FALSE(Intvl1.disjoint(Intvl3));
     EXPECT_TRUE(Intvl1.disjoint(Empty));
   }
+  {
+    // Check comesBefore().
+    sandboxir::Interval<sandboxir::Instruction> Intvl1(I0, I0);
+    sandboxir::Interval<sandboxir::Instruction> Intvl2(I2, I2);
+    EXPECT_TRUE(Intvl1.comesBefore(Intvl2));
+    EXPECT_FALSE(Intvl2.comesBefore(Intvl1));
+
+    sandboxir::Interval<sandboxir::Instruction> Intvl12(I1, I2);
+    EXPECT_TRUE(Intvl1.comesBefore(Intvl12));
+    EXPECT_FALSE(Intvl12.comesBefore(Intvl1));
+    {
+#ifndef NDEBUG
+      // Check comesBefore() with non-disjoint intervals.
+      sandboxir::Interval<sandboxir::Instruction> Intvl1(I0, I2);
+      sandboxir::Interval<sandboxir::Instruction> Intvl2(I2, I2);
+      EXPECT_DEATH(Intvl1.comesBefore(Intvl2), ".*disjoint.*");
+#endif // NDEBUG
+    }
+  }
 }
 
 // Helper function for returning a vector of instruction pointers from a range

@vporpo vporpo merged commit 31b85c6 into llvm:main Oct 11, 2024
9 of 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