[SandboxVec] Simple Instruction Interval class #108882
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-transforms Author: vporpo (vporpo) ChangesAn InstrInterval is a range of instructions in a block. The class will eventually have an API for set operations, like union, intersection etc. Full diff: https://github.com/llvm/llvm-project/pull/108882.diff 4 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/InstrInterval.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/InstrInterval.h
new file mode 100644
index 00000000000000..9ba451aebe3f7f
--- /dev/null
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/InstrInterval.h
@@ -0,0 +1,103 @@
+//===- InstrInterval.h ------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_INSTRINTERVAL_H
+#define LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_INSTRINTERVAL_H
+
+#include "llvm/SandboxIR/SandboxIR.h"
+#include <iterator>
+
+namespace llvm::sandboxir {
+
+/// A simple iterator for iterating the interval.
+template <typename DerefType, typename InstrIntervalType>
+class InstrIntervalIterator {
+ sandboxir::Instruction *I;
+ InstrIntervalType &R;
+
+public:
+ using difference_type = std::ptrdiff_t;
+ using value_type = sandboxir::Instruction;
+ using pointer = value_type *;
+ using reference = sandboxir::Instruction &;
+ using iterator_category = std::bidirectional_iterator_tag;
+
+ InstrIntervalIterator(sandboxir::Instruction *I, InstrIntervalType &R)
+ : I(I), R(R) {}
+ bool operator==(const InstrIntervalIterator &Other) const {
+ assert(&R == &Other.R && "Iterators belong to different regions!");
+ return Other.I == I;
+ }
+ bool operator!=(const InstrIntervalIterator &Other) const {
+ return !(*this == Other);
+ }
+ InstrIntervalIterator &operator++() {
+ assert(I != nullptr && "already at end()!");
+ I = I->getNextNode();
+ return *this;
+ }
+ InstrIntervalIterator operator++(int) {
+ auto ItCopy = *this;
+ ++*this;
+ return ItCopy;
+ }
+ InstrIntervalIterator &operator--() {
+ // `I` is nullptr for end() when ToI is the BB terminator.
+ I = I != nullptr ? I->getPrevNode() : R.ToI;
+ return *this;
+ }
+ InstrIntervalIterator operator--(int) {
+ auto ItCopy = *this;
+ --*this;
+ return ItCopy;
+ }
+ template <typename T =
+ std::enable_if<std::is_same<DerefType, Instruction *&>::value>>
+ sandboxir::Instruction &operator*() {
+ return *I;
+ }
+ DerefType operator*() const { return *I; }
+};
+
+class InstrInterval {
+ Instruction *FromI;
+ Instruction *ToI;
+
+public:
+ InstrInterval() : FromI(nullptr), ToI(nullptr) {}
+ InstrInterval(Instruction *FromI, Instruction *ToI)
+ : FromI(FromI), ToI(ToI) {}
+ bool empty() const {
+ assert(((FromI == nullptr && ToI == nullptr) ||
+ (FromI != nullptr && ToI != nullptr)) &&
+ "Either none or both should be null");
+ return FromI == nullptr;
+ }
+ bool contains(Instruction *I) const {
+ if (empty())
+ return false;
+ return (FromI == I || FromI->comesBefore(I)) &&
+ (I == ToI || I->comesBefore(ToI));
+ }
+
+ using iterator =
+ InstrIntervalIterator<sandboxir::Instruction &, InstrInterval>;
+ using const_iterator = InstrIntervalIterator<const sandboxir::Instruction &,
+ const InstrInterval>;
+ iterator begin() { return iterator(FromI, *this); }
+ iterator end() {
+ return iterator(ToI != nullptr ? ToI->getNextNode() : nullptr, *this);
+ }
+ const_iterator begin() const { return const_iterator(FromI, *this); }
+ const_iterator end() const {
+ return const_iterator(ToI != nullptr ? ToI->getNextNode() : nullptr, *this);
+ }
+};
+} // namespace llvm::sandboxir
+
+#endif // LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_INSTRINTERVAL_H
diff --git a/llvm/unittests/Transforms/Vectorize/CMakeLists.txt b/llvm/unittests/Transforms/Vectorize/CMakeLists.txt
index 1354558a94f0d5..0df39c41a90414 100644
--- a/llvm/unittests/Transforms/Vectorize/CMakeLists.txt
+++ b/llvm/unittests/Transforms/Vectorize/CMakeLists.txt
@@ -1,3 +1,5 @@
+add_subdirectory(SandboxVectorizer)
+
set(LLVM_LINK_COMPONENTS
Analysis
Core
diff --git a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/CMakeLists.txt b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/CMakeLists.txt
new file mode 100644
index 00000000000000..22ac8eb793d195
--- /dev/null
+++ b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/CMakeLists.txt
@@ -0,0 +1,12 @@
+set(LLVM_LINK_COMPONENTS
+ Analysis
+ Core
+ Vectorize
+ AsmParser
+ TargetParser
+ SandboxIR
+ )
+
+add_llvm_unittest(SandboxVectorizerTests
+ InstrIntervalTest.cpp
+ )
diff --git a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/InstrIntervalTest.cpp b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/InstrIntervalTest.cpp
new file mode 100644
index 00000000000000..b8b8e4e982be56
--- /dev/null
+++ b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/InstrIntervalTest.cpp
@@ -0,0 +1,68 @@
+//===- InstrIntervalTest.cpp ----------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/InstrInterval.h"
+#include "llvm/AsmParser/Parser.h"
+#include "llvm/SandboxIR/SandboxIR.h"
+#include "llvm/Support/SourceMgr.h"
+#include "gmock/gmock-matchers.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+struct InstrIntervalTest : public testing::Test {
+ LLVMContext C;
+ std::unique_ptr<Module> M;
+
+ void parseIR(LLVMContext &C, const char *IR) {
+ SMDiagnostic Err;
+ M = parseAssemblyString(IR, Err, C);
+ if (!M)
+ Err.print("InstrIntervalTest", errs());
+ }
+};
+
+TEST_F(InstrIntervalTest, Basic) {
+ parseIR(C, R"IR(
+define void @foo(i8 %v0) {
+ %add0 = add i8 %v0, %v0
+ %add1 = add i8 %v0, %v0
+ %add2 = add i8 %v0, %v0
+ ret void
+}
+)IR");
+ Function &LLVMF = *M->getFunction("foo");
+ sandboxir::Context Ctx(C);
+ auto &F = *Ctx.createFunction(&LLVMF);
+ auto *BB = &*F.begin();
+ auto It = BB->begin();
+ auto *I0 = &*It++;
+ auto *I1 = &*It++;
+ auto *I2 = &*It++;
+ auto *Ret = &*It++;
+
+ sandboxir::InstrInterval Interval(I0, Ret);
+ // Check empty().
+ EXPECT_FALSE(Interval.empty());
+ sandboxir::InstrInterval Empty;
+ EXPECT_TRUE(Empty.empty());
+ sandboxir::InstrInterval One(I0, I0);
+ EXPECT_FALSE(One.empty());
+ // Check contains().
+ for (auto &I : *BB) {
+ EXPECT_TRUE(Interval.contains(&I));
+ EXPECT_FALSE(Empty.contains(&I));
+ }
+ EXPECT_FALSE(One.contains(I1));
+ EXPECT_FALSE(One.contains(I2));
+ EXPECT_FALSE(One.contains(Ret));
+ // Check iterator.
+ auto BBIt = BB->begin();
+ for (auto &I : Interval)
+ EXPECT_EQ(&I, &*BBIt++);
+}
|
|
Same comments as for the dependency graph:
|
Do you mean like adding more comments?
I think of it more like a helper class for the vectorizer, rather than an analysis. It's not specific to the vectorizer, so it would make sense to move it into Regarding the Dependency Graph, that class is specific to the vectorizer. It's not going to be a generic Dependency Graph that others could use. |
|
could use some doxygen Yep. At the top of the file. |
|
Like an overview. Especially the dependency graph could benefit from an introduction/overview. |
vporpo
force-pushed
the
Interval
branch
2 times, most recently
from
a96c9f7 to
3f7a8e6
Compare
aeubanks
reviewed
Sep 18, 2024
| public: | ||
| InstrInterval() : FromI(nullptr), ToI(nullptr) {} | ||
| InstrInterval(Instruction *FromI, Instruction *ToI) | ||
| : FromI(FromI), ToI(ToI) {} |
There was a problem hiding this comment.
assert(FromI->comesBefore(ToI))?
There was a problem hiding this comment.
It can also contain a single instruction so it's assert(FromI == ToI || FromI->comesBefore(ToI)).
An InstrInterval is a range of instructions in a block. The class will eventually have an API for set operations, like union, intersection etc.
|
ping |
aeubanks
approved these changes
Sep 19, 2024
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/119/builds/2237 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/146/builds/1198 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
An InstrInterval is a range of instructions in a block. The class will eventually have an API for set operations, like union, intersection etc.