-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Revert "[SandboxVec] Add barebones Region class." #109058
Merged
Merged
Conversation
This file contains 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
This reverts commit 3aecf41.
@llvm/pr-subscribers-llvm-transforms Author: Jorge Gorbe Moya (slackito) ChangesReverts llvm/llvm-project#108899 It broke the llvm-clang-x86_64-win-fast buildbot. Full diff: https://github.com/llvm/llvm-project/pull/109058.diff 5 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Region.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Region.h
deleted file mode 100644
index 71d9ab58250993..00000000000000
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Region.h
+++ /dev/null
@@ -1,106 +0,0 @@
-//===- Region.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_REGION_H
-#define LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_REGION_H
-
-#include "llvm/ADT/SetVector.h"
-#include "llvm/ADT/iterator_range.h"
-#include "llvm/SandboxIR/SandboxIR.h"
-#include "llvm/Support/InstructionCost.h"
-#include "llvm/Support/raw_ostream.h"
-
-namespace llvm::sandboxir {
-
-/// The main job of the Region is to point to new instructions generated by
-/// vectorization passes. It is the unit that RegionPasses operate on with their
-/// runOnRegion() function.
-///
-/// The region allows us to stack transformations horizontally, meaning that
-/// each transformation operates on a single region and the resulting region is
-/// the input to the next transformation, as opposed to vertically, which is the
-/// common way of applying a transformation across the whole BB. This enables us
-/// to check for profitability and decide whether we accept or rollback at a
-/// region granularity, which is much better than doing this at the BB level.
-///
-// Traditional approach: transformations applied vertically for the whole BB
-// BB
-// +----+
-// | |
-// | |
-// | | -> Transform1 -> ... -> TransformN -> Check Cost
-// | |
-// | |
-// +----+
-//
-// Region-based approach: transformations applied horizontally, for each Region
-// BB
-// +----+
-// |Rgn1| -> Transform1 -> ... -> TransformN -> Check Cost
-// | |
-// |Rgn2| -> Transform1 -> ... -> TransformN -> Check Cost
-// | |
-// |Rgn3| -> Transform1 -> ... -> TransformN -> Check Cost
-// +----+
-
-class Region {
- /// All the instructions in the Region. Only new instructions generated during
- /// vectorization are part of the Region.
- SetVector<Instruction *> Insts;
-
- /// A unique ID, used for debugging.
- unsigned RegionID = 0;
-
- Context &Ctx;
-
- /// The basic block containing this region.
- BasicBlock &BB;
-
- // TODO: Add cost modeling.
- // TODO: Add a way to encode/decode region info to/from metadata.
-
-public:
- Region(Context &Ctx, BasicBlock &BB);
- ~Region();
-
- BasicBlock *getParent() const { return &BB; }
- Context &getContext() const { return Ctx; }
- /// Returns the region's unique ID.
- unsigned getID() const { return RegionID; }
-
- /// Adds I to the set.
- void add(Instruction *I);
- /// Removes I from the set.
- void remove(Instruction *I);
- /// Returns true if I is in the Region.
- bool contains(Instruction *I) const { return Insts.contains(I); }
- /// Returns true if the Region has no instructions.
- bool empty() const { return Insts.empty(); }
-
- using iterator = decltype(Insts.begin());
- iterator begin() { return Insts.begin(); }
- iterator end() { return Insts.end(); }
- iterator_range<iterator> insts() { return make_range(begin(), end()); }
-
-#ifndef NDEBUG
- /// This is an expensive check, meant for testing.
- bool operator==(const Region &Other) const;
- bool operator!=(const Region &other) const { return !(*this == other); }
-
- void dump(raw_ostream &OS) const;
- void dump() const;
- friend raw_ostream &operator<<(raw_ostream &OS, const Region &Rgn) {
- Rgn.dump(OS);
- return OS;
- }
-#endif
-};
-
-} // namespace llvm::sandboxir
-
-#endif // LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_REGION_H
diff --git a/llvm/lib/Transforms/Vectorize/CMakeLists.txt b/llvm/lib/Transforms/Vectorize/CMakeLists.txt
index f33906b05fedd1..59d04ac3cecd00 100644
--- a/llvm/lib/Transforms/Vectorize/CMakeLists.txt
+++ b/llvm/lib/Transforms/Vectorize/CMakeLists.txt
@@ -5,7 +5,6 @@ add_llvm_component_library(LLVMVectorize
LoopVectorize.cpp
SandboxVectorizer/DependencyGraph.cpp
SandboxVectorizer/Passes/BottomUpVec.cpp
- SandboxVectorizer/Region.cpp
SandboxVectorizer/SandboxVectorizer.cpp
SLPVectorizer.cpp
Vectorize.cpp
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Region.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Region.cpp
deleted file mode 100644
index 0c5d66cdeb8f40..00000000000000
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Region.cpp
+++ /dev/null
@@ -1,46 +0,0 @@
-//===- Region.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/Region.h"
-
-namespace llvm::sandboxir {
-
-Region::Region(Context &Ctx, BasicBlock &BB) : Ctx(Ctx), BB(BB) {
- static unsigned StaticRegionID;
- RegionID = StaticRegionID++;
-}
-
-Region::~Region() {}
-
-void Region::add(Instruction *I) { Insts.insert(I); }
-
-void Region::remove(Instruction *I) { Insts.remove(I); }
-
-#ifndef NDEBUG
-bool Region::operator==(const Region &Other) const {
- if (Insts.size() != Other.Insts.size())
- return false;
- if (!std::is_permutation(Insts.begin(), Insts.end(), Other.Insts.begin()))
- return false;
- return true;
-}
-
-void Region::dump(raw_ostream &OS) const {
- OS << "RegionID: " << getID() << "\n";
- for (auto *I : Insts)
- OS << *I << "\n";
-}
-
-void Region::dump() const {
- dump(dbgs());
- dbgs() << "\n";
-}
-
-} // namespace llvm::sandboxir
-
-#endif // NDEBUG
diff --git a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/CMakeLists.txt b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/CMakeLists.txt
index 23dbe7f46bc995..488c9c2344b56c 100644
--- a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/CMakeLists.txt
+++ b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/CMakeLists.txt
@@ -9,5 +9,4 @@ set(LLVM_LINK_COMPONENTS
add_llvm_unittest(SandboxVectorizerTests
DependencyGraphTest.cpp
- RegionTest.cpp
)
diff --git a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/RegionTest.cpp b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/RegionTest.cpp
deleted file mode 100644
index 85b567ed51cd8a..00000000000000
--- a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/RegionTest.cpp
+++ /dev/null
@@ -1,82 +0,0 @@
-//===- RegionTest.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/Region.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 RegionTest : 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("RegionTest", errs());
- }
-};
-
-TEST_F(RegionTest, Basic) {
- parseIR(C, R"IR(
-define i8 @foo(i8 %v0, i8 %v1) {
- %t0 = add i8 %v0, 1
- %t1 = add i8 %t0, %v1
- ret i8 %t1
-}
-)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 *T0 = cast<sandboxir::Instruction>(&*It++);
- auto *T1 = cast<sandboxir::Instruction>(&*It++);
- auto *Ret = cast<sandboxir::Instruction>(&*It++);
- sandboxir::Region Rgn(Ctx, *BB);
-
- // Check getters
- EXPECT_EQ(BB, Rgn.getParent());
- EXPECT_EQ(&Ctx, &Rgn.getContext());
- EXPECT_EQ(0U, Rgn.getID());
-
- // Check add / remove / empty.
- EXPECT_TRUE(Rgn.empty());
- Rgn.add(T0);
- EXPECT_FALSE(Rgn.empty());
- Rgn.remove(T0);
- EXPECT_TRUE(Rgn.empty());
-
- // Check iteration.
- Rgn.add(T0);
- Rgn.add(T1);
- Rgn.add(Ret);
- // Use an ordered matcher because we're supposed to preserve the insertion
- // order for determinism.
- EXPECT_THAT(Rgn.insts(), testing::ElementsAre(T0, T1, Ret));
-
- // Check contains
- EXPECT_TRUE(Rgn.contains(T0));
- Rgn.remove(T0);
- EXPECT_FALSE(Rgn.contains(T0));
-
-#ifndef NDEBUG
- // Check equality comparison. Insert in reverse order into `Other` to check
- // that comparison is order-independent.
- sandboxir::Region Other(Ctx, *BB);
- Other.add(Ret);
- EXPECT_NE(Rgn, Other);
- Other.add(T1);
- EXPECT_EQ(Rgn, Other);
-#endif
-}
|
hamphet
pushed a commit
to hamphet/llvm-project
that referenced
this pull request
Sep 18, 2024
Reverts llvm#108899 It broke the llvm-clang-x86_64-win-fast buildbot.
tmsri
pushed a commit
to tmsri/llvm-project
that referenced
this pull request
Sep 19, 2024
Reverts llvm#108899 It broke the llvm-clang-x86_64-win-fast buildbot.
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.
Reverts #108899
It broke the llvm-clang-x86_64-win-fast buildbot.