[SandboxVec] Legality boilerplate #108650
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) ChangesThis patch adds the basic API for the Legality component of the vectorizer. It also adds some very basic code in the bottom-up vectorizer that uses the API. Full diff: https://github.com/llvm/llvm-project/pull/108650.diff 6 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Legality.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Legality.h
new file mode 100644
index 00000000000000..948a16f70213f1
--- /dev/null
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Legality.h
@@ -0,0 +1,62 @@
+//===- Legality.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
+//
+//===----------------------------------------------------------------------===//
+//
+// Legality checks for the Sandbox Vectorizer.
+//
+
+#ifndef LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_LEGALITY_H
+#define LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_LEGALITY_H
+
+#include "llvm/SandboxIR/SandboxIR.h"
+
+namespace llvm::sandboxir {
+
+class Legality;
+
+enum class LegalityResultID {
+ Widen, ///> Vectorize by combining scalars to a vector.
+};
+
+/// The legality outcome is represented by a class rather than an enum class
+/// because in some cases the legality checks are expensive and look for a
+/// particular instruction that can be passed along to the vectorizer to avoid
+/// repeating the same expensive computation.
+class LegalityResult {
+protected:
+ LegalityResultID ID;
+ /// Only Legality can create LegalityResults.
+ LegalityResult(LegalityResultID ID) : ID(ID) {}
+ friend class Legality;
+
+public:
+ LegalityResultID getSubclassID() const { return ID; }
+};
+
+class Widen final : public LegalityResult {
+ friend class Legality;
+ Widen() : LegalityResult(LegalityResultID::Widen) {}
+
+public:
+ static bool classof(const LegalityResult *From) {
+ return From->getSubclassID() == LegalityResultID::Widen;
+ }
+};
+
+/// Performs the legality analysis and returns a LegalityResult object.
+class Legality {
+public:
+ Legality() = default;
+ LegalityResult canVectorize(ArrayRef<Value *> Bndl) {
+ // TODO: For now everything is legal.
+ return Widen();
+ }
+};
+
+} // namespace llvm::sandboxir
+
+#endif // LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_LEGALITY_H
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h
index 5b3d1a50aa1ec0..ba78106d1f9261 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h
@@ -12,11 +12,18 @@
#ifndef LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_PASSES_BOTTOMUPVEC_H
#define LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_PASSES_BOTTOMUPVEC_H
+#include "llvm/ADT/ArrayRef.h"
#include "llvm/SandboxIR/Pass.h"
+#include "llvm/SandboxIR/SandboxIR.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Legality.h"
namespace llvm::sandboxir {
class BottomUpVec final : public FunctionPass {
+ bool Change = false;
+ Legality Legal;
+ void vectorizeRec(ArrayRef<Value *> Bndl);
+ void tryVectorize(ArrayRef<Value *> Seeds);
public:
BottomUpVec() : FunctionPass("bottom-up-vec") {}
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
index c4870b70fd52da..e1c9382a04a8bf 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
@@ -7,7 +7,58 @@
//===----------------------------------------------------------------------===//
#include "llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h"
+#include "llvm/ADT/SmallVector.h"
using namespace llvm::sandboxir;
-bool BottomUpVec::runOnFunction(Function &F) { return false; }
+namespace llvm::sandboxir {
+// TODO: This is a temporary function that returns some seeds.
+// Replace this with SeedCollector's function when it lands.
+static llvm::SmallVector<Value *, 4> collectSeeds(BasicBlock &BB) {
+ llvm::SmallVector<Value *, 4> Seeds;
+ for (auto &I : BB)
+ if (auto *SI = llvm::dyn_cast<StoreInst>(&I))
+ Seeds.push_back(SI);
+ return Seeds;
+}
+
+static SmallVector<Value *, 4> getOperand(ArrayRef<Value *> Bndl,
+ unsigned OpIdx) {
+ SmallVector<Value *, 4> Operands;
+ for (Value *BndlV : Bndl) {
+ auto *BndlI = cast<Instruction>(BndlV);
+ Operands.push_back(BndlI->getOperand(OpIdx));
+ }
+ return Operands;
+}
+
+} // namespace llvm::sandboxir
+
+void BottomUpVec::vectorizeRec(ArrayRef<Value *> Bndl) {
+ auto LegalityRes = Legal.canVectorize(Bndl);
+ switch (LegalityRes.getSubclassID()) {
+ case LegalityResultID::Widen: {
+ auto *I = cast<Instruction>(Bndl[0]);
+ for (auto OpIdx : seq<unsigned>(I->getNumOperands())) {
+ auto OperandBndl = getOperand(Bndl, OpIdx);
+ vectorizeRec(OperandBndl);
+ }
+ break;
+ }
+ }
+}
+
+void BottomUpVec::tryVectorize(ArrayRef<Value *> Bndl) { vectorizeRec(Bndl); }
+
+bool BottomUpVec::runOnFunction(Function &F) {
+ Change = false;
+ // TODO: Start from innermost BBs first
+ for (auto &BB : F) {
+ // TODO: Replace with proper SeedCollector function.
+ auto Seeds = collectSeeds(BB);
+ // TODO: Slice Seeds into smaller chunks.
+ if (Seeds.size() >= 2)
+ tryVectorize(Seeds);
+ }
+ return Change;
+}
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..7e57977e3e80e9
--- /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
+ LegalityTest.cpp
+ )
diff --git a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/LegalityTest.cpp b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/LegalityTest.cpp
new file mode 100644
index 00000000000000..89e1e9ad4449b7
--- /dev/null
+++ b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/LegalityTest.cpp
@@ -0,0 +1,56 @@
+//===- LegalityTest.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/Legality.h"
+#include "llvm/AsmParser/Parser.h"
+#include "llvm/SandboxIR/SandboxIR.h"
+#include "llvm/Support/SourceMgr.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+struct LegalityTest : 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("LegalityTest", errs());
+ }
+};
+
+TEST_F(LegalityTest, Legality) {
+ parseIR(C, R"IR(
+define void @foo(ptr %ptr) {
+ %gep0 = getelementptr float, ptr %ptr, i32 0
+ %gep1 = getelementptr float, ptr %ptr, i32 1
+ %ld0 = load float, ptr %gep0
+ %ld1 = load float, ptr %gep0
+ store float %ld0, ptr %gep0
+ store float %ld1, ptr %gep1
+ ret void
+}
+)IR");
+ llvm::Function *LLVMF = &*M->getFunction("foo");
+ sandboxir::Context Ctx(C);
+ auto *F = Ctx.createFunction(LLVMF);
+ auto *BB = &*F->begin();
+ auto It = BB->begin();
+ [[maybe_unused]] auto *Gep0 = cast<sandboxir::GetElementPtrInst>(&*It++);
+ [[maybe_unused]] auto *Gep1 = cast<sandboxir::GetElementPtrInst>(&*It++);
+ [[maybe_unused]] auto *Ld0 = cast<sandboxir::LoadInst>(&*It++);
+ [[maybe_unused]] auto *Ld1 = cast<sandboxir::LoadInst>(&*It++);
+ auto *St0 = cast<sandboxir::StoreInst>(&*It++);
+ auto *St1 = cast<sandboxir::StoreInst>(&*It++);
+
+ sandboxir::Legality Legal;
+ auto Result = Legal.canVectorize({St0, St1});
+ EXPECT_TRUE(isa<sandboxir::Widen>(Result));
+}
|
tschuett
reviewed
Sep 14, 2024
| class Legality { | ||
| public: | ||
| Legality() = default; | ||
| LegalityResult canVectorize(ArrayRef<Value *> Bndl) { |
There was a problem hiding this comment.
You probably are only upstreaming a subset of the full version, but how can I state:
I cannot vectorize the values.
There was a problem hiding this comment.
And how can I state that this set of values can be vectorized in two different ways.
There was a problem hiding this comment.
Yeah, this is a very limited version. The full version will tell you whether this is legal or not and also how to vectorize the values.
There was a problem hiding this comment.
In the stripped down version, it's best to use the default case as "don't vectorize anything" (i.e. conservative), instead of "vectorize everything".
There was a problem hiding this comment.
Yeah, we will change this once we implement more components.
tschuett
reviewed
Sep 14, 2024
| }; | ||
|
|
||
| /// Performs the legality analysis and returns a LegalityResult object. | ||
| class Legality { |
There was a problem hiding this comment.
I would have expected something ala LegalityCheck or LegalityAnalysis .
There was a problem hiding this comment.
LegalityAnalysis is a better name indeed.
tmsri
reviewed
Sep 17, 2024
tmsri
approved these changes
Sep 17, 2024
This patch adds the basic API for the Legality component of the vectorizer. It also adds some very basic code in the bottom-up vectorizer that uses the API.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/30/builds/6395 Here is the relevant piece of the build log for the reference |
hamphet
pushed a commit
to hamphet/llvm-project
that referenced
this pull request
Sep 18, 2024
This patch adds the basic API for the Legality component of the vectorizer. It also adds some very basic code in the bottom-up vectorizer that uses the API.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/17/builds/2743 Here is the relevant piece of the build log for the reference |
|
Hi, I noticed that the following starts crashing with this patch: It fails like |
alinas
reviewed
Sep 18, 2024
| class LegalityResult { | ||
| protected: | ||
| LegalityResultID ID; | ||
| /// Only Legality can create LegalityResults. |
There was a problem hiding this comment.
Update comment to LegalityAnalysis
| auto LegalityRes = Legality.canVectorize(Bndl); | ||
| switch (LegalityRes.getSubclassID()) { | ||
| case LegalityResultID::Widen: { | ||
| auto *I = cast<Instruction>(Bndl[0]); |
There was a problem hiding this comment.
This cast is incorrect. After the first recursive call, the operands are not necessary instructions.
There was a problem hiding this comment.
If legality decides that it's legal to vectorize, then it should be an instruction.
There was a problem hiding this comment.
The crash I reported in
#108650 (comment)
crashes exactly on this cast.
Then Bndl[0] is
i16 0 ; SB5. (ConstantInt)
There was a problem hiding this comment.
@mikaelholmen Yeah the pass is not working yet, so it will be crashing for a while. Should I add a flag to disable it by default so that we don't block you ?
There was a problem hiding this comment.
No, not needed. I've disabled fuzzy testing of the pass downstream so it's ok.
| class Legality { | ||
| public: | ||
| Legality() = default; | ||
| LegalityResult canVectorize(ArrayRef<Value *> Bndl) { |
There was a problem hiding this comment.
In the stripped down version, it's best to use the default case as "don't vectorize anything" (i.e. conservative), instead of "vectorize everything".
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/146/builds/1180 Here is the relevant piece of the build log for the reference |
tmsri
pushed a commit
to tmsri/llvm-project
that referenced
this pull request
Sep 19, 2024
This patch adds the basic API for the Legality component of the vectorizer. It also adds some very basic code in the bottom-up vectorizer that uses the API.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/456 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.
This patch adds the basic API for the Legality component of the vectorizer. It also adds some very basic code in the bottom-up vectorizer that uses the API.