Skip to content

Conversation

vporpo
Copy link

@vporpo vporpo commented Sep 20, 2024

The new constructor creates an InstrInterval from an ArrayRef<Instruction *>. This patch also adds top() and bottom() getters.

@llvmbot
Copy link
Member

llvmbot commented Sep 20, 2024

@llvm/pr-subscribers-llvm-transforms

Author: vporpo (vporpo)

Changes

The new constructor creates an InstrInterval from an ArrayRef<Instruction *>. This patch also adds top() and bottom() getters.


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

2 Files Affected:

  • (modified) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/InstrInterval.h (+13)
  • (modified) llvm/unittests/Transforms/Vectorize/SandboxVectorizer/InstrIntervalTest.cpp (+20)
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/InstrInterval.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/InstrInterval.h
index dcf9ca00ba11ac..d9ad3874ef6e33 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/InstrInterval.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/InstrInterval.h
@@ -80,6 +80,17 @@ class InstrInterval {
     assert((FromI == ToI || FromI->comesBefore(ToI)) &&
            "FromI should come before TopI!");
   }
+  InstrInterval(ArrayRef<Instruction *> Instrs) {
+    assert(!Instrs.empty() && "Expected non-empty Instrs!");
+    FromI = Instrs[0];
+    ToI = Instrs[0];
+    for (auto *I : drop_begin(Instrs)) {
+      if (I->comesBefore(FromI))
+        FromI = I;
+      if (ToI->comesBefore(I))
+        ToI = I;
+    }
+  }
   bool empty() const {
     assert(((FromI == nullptr && ToI == nullptr) ||
             (FromI != nullptr && ToI != nullptr)) &&
@@ -92,6 +103,8 @@ class InstrInterval {
     return (FromI == I || FromI->comesBefore(I)) &&
            (I == ToI || I->comesBefore(ToI));
   }
+  Instruction *top() const { return FromI; }
+  Instruction *bottom() const { return ToI; }
 
   using iterator =
       InstrIntervalIterator<sandboxir::Instruction &, InstrInterval>;
diff --git a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/InstrIntervalTest.cpp b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/InstrIntervalTest.cpp
index 9553158150525b..e22bb78a07d300 100644
--- a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/InstrIntervalTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/InstrIntervalTest.cpp
@@ -50,6 +50,26 @@ define void @foo(i8 %v0) {
 #ifndef NDEBUG
   EXPECT_DEATH(sandboxir::InstrInterval(I1, I0), ".*before.*");
 #endif // NDEBUG
+  // Check InstrInterval(ArrayRef), from(), to().
+  {
+    sandboxir::InstrInterval Interval(
+        SmallVector<sandboxir::Instruction *>({I0, Ret}));
+    EXPECT_EQ(Interval.top(), I0);
+    EXPECT_EQ(Interval.bottom(), Ret);
+  }
+  {
+    sandboxir::InstrInterval Interval(
+        SmallVector<sandboxir::Instruction *>({Ret, I0}));
+    EXPECT_EQ(Interval.top(), I0);
+    EXPECT_EQ(Interval.bottom(), Ret);
+  }
+  {
+    sandboxir::InstrInterval Interval(
+        SmallVector<sandboxir::Instruction *>({I0, I0}));
+    EXPECT_EQ(Interval.top(), I0);
+    EXPECT_EQ(Interval.bottom(), I0);
+  }
+
   // Check empty().
   EXPECT_FALSE(Interval.empty());
   sandboxir::InstrInterval Empty;

for (auto *I : drop_begin(Instrs)) {
if (I->comesBefore(FromI))
FromI = I;
if (ToI->comesBefore(I))
Copy link
Member

Choose a reason for hiding this comment

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

else if here? Either FromI moves or ToI moves but not both.

Copy link
Author

Choose a reason for hiding this comment

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

I think you are right. Initially they both have the same value and from that point on FromI will always be before ToI, so if I comes before FromI it will never be after ToI

The new constructor creates an InstrInterval from an ArrayRef<Instruction *>.
This patch also adds top() and bottom() getters.
@vporpo vporpo merged commit abc2412 into llvm:main Sep 20, 2024
5 of 7 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