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][Legality] Reject non-instructions #113190

Merged
merged 1 commit into from
Oct 25, 2024
Merged

Conversation

vporpo
Copy link
Contributor

@vporpo vporpo commented Oct 21, 2024

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Oct 21, 2024

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-vectorizers

Author: vporpo (vporpo)

Changes

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

4 Files Affected:

  • (modified) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Legality.h (+9-1)
  • (modified) llvm/lib/Transforms/Vectorize/SandboxVectorizer/Legality.cpp (+17-1)
  • (modified) llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp (+1-1)
  • (modified) llvm/unittests/Transforms/Vectorize/SandboxVectorizer/LegalityTest.cpp (+12-1)
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Legality.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Legality.h
index 233abf3efd64e1..11d72ff6b771e6 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Legality.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Legality.h
@@ -28,6 +28,7 @@ enum class LegalityResultID {
 
 /// The reason for vectorizing or not vectorizing.
 enum class ResultReason {
+  NotInstructions,
   DiffOpcodes,
   DiffTypes,
 };
@@ -45,6 +46,8 @@ struct ToStr {
 
   static const char *getVecReason(ResultReason Reason) {
     switch (Reason) {
+    case ResultReason::NotInstructions:
+      return "NotInstructions";
     case ResultReason::DiffOpcodes:
       return "DiffOpcodes";
     case ResultReason::DiffTypes:
@@ -65,6 +68,10 @@ class LegalityResult {
   LegalityResult(LegalityResultID ID) : ID(ID) {}
   friend class LegalityAnalysis;
 
+  /// We shouldn't need copies.
+  LegalityResult(const LegalityResult &) = delete;
+  LegalityResult &operator=(const LegalityResult &) = delete;
+
 public:
   virtual ~LegalityResult() {}
   LegalityResultID getSubclassID() const { return ID; }
@@ -88,6 +95,7 @@ class LegalityResultWithReason : public LegalityResult {
   friend class Pack; // For constructor.
 
 public:
+  ResultReason getReason() const { return Reason; }
 #ifndef NDEBUG
   void print(raw_ostream &OS) const override {
     LegalityResult::print(OS);
@@ -136,7 +144,7 @@ class LegalityAnalysis {
   }
   /// Checks if it's legal to vectorize the instructions in \p Bndl.
   /// \Returns a LegalityResult object owned by LegalityAnalysis.
-  LegalityResult &canVectorize(ArrayRef<Value *> Bndl);
+  const LegalityResult &canVectorize(ArrayRef<Value *> Bndl);
 };
 
 } // namespace llvm::sandboxir
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Legality.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Legality.cpp
index 0e2cd83c37b0cd..f1c4577cece78a 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Legality.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Legality.cpp
@@ -7,11 +7,15 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/Vectorize/SandboxVectorizer/Legality.h"
+#include "llvm/SandboxIR/Instruction.h"
+#include "llvm/SandboxIR/Utils.h"
 #include "llvm/SandboxIR/Value.h"
 #include "llvm/Support/Debug.h"
 
 namespace llvm::sandboxir {
 
+#define DEBUG_TYPE "SBVec:Legality"
+
 #ifndef NDEBUG
 void LegalityResult::dump() const {
   print(dbgs());
@@ -26,7 +30,19 @@ LegalityAnalysis::notVectorizableBasedOnOpcodesAndTypes(
   return std::nullopt;
 }
 
-LegalityResult &LegalityAnalysis::canVectorize(ArrayRef<Value *> Bndl) {
+static void dumpBndl(ArrayRef<Value *> Bndl) {
+  for (auto *V : Bndl)
+    dbgs() << *V << "\n";
+}
+
+const LegalityResult &LegalityAnalysis::canVectorize(ArrayRef<Value *> Bndl) {
+  // If Bndl contains values other than instructions, we need to Pack.
+  if (any_of(Bndl, [](auto *V) { return !isa<Instruction>(V); })) {
+    LLVM_DEBUG(dbgs() << "Not vectorizing: Not Instructions:\n";
+               dumpBndl(Bndl););
+    return createLegalityResult<Pack>(ResultReason::NotInstructions);
+  }
+
   if (auto ReasonOpt = notVectorizableBasedOnOpcodesAndTypes(Bndl))
     return createLegalityResult<Pack>(*ReasonOpt);
 
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
index f11420e47f3e1f..ede41cd661b559 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
@@ -40,7 +40,7 @@ static SmallVector<Value *, 4> getOperand(ArrayRef<Value *> Bndl,
 }
 
 void BottomUpVec::vectorizeRec(ArrayRef<Value *> Bndl) {
-  auto LegalityRes = Legality.canVectorize(Bndl);
+  const auto &LegalityRes = Legality.canVectorize(Bndl);
   switch (LegalityRes.getSubclassID()) {
   case LegalityResultID::Widen: {
     auto *I = cast<Instruction>(Bndl[0]);
diff --git a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/LegalityTest.cpp b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/LegalityTest.cpp
index 76e5a5ce5aed92..56c6bf5f1ef1f5 100644
--- a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/LegalityTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/LegalityTest.cpp
@@ -52,8 +52,16 @@ define void @foo(ptr %ptr) {
   auto *St1 = cast<sandboxir::StoreInst>(&*It++);
 
   sandboxir::LegalityAnalysis Legality;
-  auto Result = Legality.canVectorize({St0, St1});
+  const auto &Result = Legality.canVectorize({St0, St1});
   EXPECT_TRUE(isa<sandboxir::Widen>(Result));
+
+  {
+    // Check NotInstructions
+    auto &Result = Legality.canVectorize({F, St0});
+    EXPECT_TRUE(isa<sandboxir::Pack>(Result));
+    EXPECT_EQ(cast<sandboxir::Pack>(Result).getReason(),
+              sandboxir::ResultReason::NotInstructions);
+  }
 }
 
 #ifndef NDEBUG
@@ -68,6 +76,9 @@ TEST_F(LegalityTest, LegalityResultDump) {
   sandboxir::LegalityAnalysis Legality;
   EXPECT_TRUE(
       Matches(Legality.createLegalityResult<sandboxir::Widen>(), "Widen"));
+  EXPECT_TRUE(Matches(Legality.createLegalityResult<sandboxir::Pack>(
+                          sandboxir::ResultReason::NotInstructions),
+                      "Pack Reason: NotInstructions"));
   EXPECT_TRUE(Matches(Legality.createLegalityResult<sandboxir::Pack>(
                           sandboxir::ResultReason::DiffOpcodes),
                       "Pack Reason: DiffOpcodes"));

@vporpo vporpo merged commit 6c9bbbc into llvm:main Oct 25, 2024
11 checks passed
vporpo added a commit that referenced this pull request Oct 25, 2024
@llvm-ci
Copy link
Collaborator

llvm-ci commented Oct 25, 2024

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-win-fast running on as-builder-3 while building llvm at step 6 "build-unified-tree".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/2/builds/9630

Here is the relevant piece of the build log for the reference
Step 6 (build-unified-tree) failure: build (failure)
...
[2519/4071] Building CXX object lib\AsmParser\CMakeFiles\LLVMAsmParser.dir\LLParser.cpp.obj
[2520/4071] Building CXX object lib\ProfileData\Coverage\CMakeFiles\LLVMCoverage.dir\CoverageMappingReader.cpp.obj
[2521/4071] Building CXX object lib\Passes\CMakeFiles\LLVMPasses.dir\PassBuilderBindings.cpp.obj
[2522/4071] Building CXX object lib\Passes\CMakeFiles\LLVMPasses.dir\CodeGenPassBuilder.cpp.obj
[2523/4071] Building CXX object lib\TextAPI\CMakeFiles\LLVMTextAPI.dir\SymbolSet.cpp.obj
[2524/4071] Building CXX object lib\Passes\CMakeFiles\LLVMPasses.dir\StandardInstrumentations.cpp.obj
[2525/4071] Building CXX object lib\TextAPI\CMakeFiles\LLVMTextAPI.dir\Symbol.cpp.obj
[2526/4071] Building CXX object lib\TextAPI\CMakeFiles\LLVMTextAPI.dir\RecordsSlice.cpp.obj
[2527/4071] Building CXX object lib\TextAPI\CMakeFiles\LLVMTextAPI.dir\Architecture.cpp.obj
[2528/4071] Building CXX object lib\Transforms\Vectorize\CMakeFiles\LLVMVectorize.dir\SandboxVectorizer\Legality.cpp.obj
FAILED: lib/Transforms/Vectorize/CMakeFiles/LLVMVectorize.dir/SandboxVectorizer/Legality.cpp.obj 
C:\ninja\ccache.exe C:\PROGRA~1\MICROS~2\2022\COMMUN~1\VC\Tools\MSVC\1438~1.331\bin\Hostx64\x64\cl.exe  /nologo /TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -IC:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\build\lib\Transforms\Vectorize -IC:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\lib\Transforms\Vectorize -IC:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\build\include -IC:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include /DWIN32 /D_WINDOWS   /Zc:inline /Zc:preprocessor /Zc:__cplusplus /Oi /bigobj /permissive- /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd5105 -wd4324 -wd4251 -wd4275 -w14062 -we4238 /Gw /O2 /Ob2 /DNDEBUG -MD  /EHs-c- /GR- -std:c++17 /showIncludes /Folib\Transforms\Vectorize\CMakeFiles\LLVMVectorize.dir\SandboxVectorizer\Legality.cpp.obj /Fdlib\Transforms\Vectorize\CMakeFiles\LLVMVectorize.dir\LLVMVectorize.pdb /FS -c C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\lib\Transforms\Vectorize\SandboxVectorizer\Legality.cpp
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\lib\Transforms\Vectorize\SandboxVectorizer\Legality.cpp(35): error C2679: binary '<<': no operator found which takes a right-hand operand of type 'llvm::sandboxir::Value' (or there is no acceptable conversion)
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Support/raw_ostream.h(319): note: could be 'llvm::raw_ostream &llvm::raw_ostream::operator <<(const llvm::FormattedBytes &)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Support/raw_ostream.h(316): note: or       'llvm::raw_ostream &llvm::raw_ostream::operator <<(const llvm::formatv_object_base &)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Support/raw_ostream.h(313): note: or       'llvm::raw_ostream &llvm::raw_ostream::operator <<(const llvm::FormattedNumber &)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Support/raw_ostream.h(310): note: or       'llvm::raw_ostream &llvm::raw_ostream::operator <<(const llvm::FormattedString &)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Support/raw_ostream.h(307): note: or       'llvm::raw_ostream &llvm::raw_ostream::operator <<(const llvm::format_object_base &)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Support/raw_ostream.h(293): note: or       'llvm::raw_ostream &llvm::raw_ostream::operator <<(llvm::raw_ostream::Colors)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Support/raw_ostream.h(287): note: or       'llvm::raw_ostream &llvm::raw_ostream::operator <<(double)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Support/raw_ostream.h(283): note: or       'llvm::raw_ostream &llvm::raw_ostream::operator <<(int)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Support/raw_ostream.h(279): note: or       'llvm::raw_ostream &llvm::raw_ostream::operator <<(unsigned int)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Support/raw_ostream.h(277): note: or       'llvm::raw_ostream &llvm::raw_ostream::operator <<(const void *)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Support/raw_ostream.h(276): note: or       'llvm::raw_ostream &llvm::raw_ostream::operator <<(__int64)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Support/raw_ostream.h(275): note: or       'llvm::raw_ostream &llvm::raw_ostream::operator <<(unsigned __int64)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Support/raw_ostream.h(274): note: or       'llvm::raw_ostream &llvm::raw_ostream::operator <<(long)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Support/raw_ostream.h(273): note: or       'llvm::raw_ostream &llvm::raw_ostream::operator <<(unsigned long)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Support/raw_ostream.h(269): note: or       'llvm::raw_ostream &llvm::raw_ostream::operator <<(const llvm::SmallVectorImpl<char> &)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Support/raw_ostream.h(265): note: or       'llvm::raw_ostream &llvm::raw_ostream::operator <<(const std::string_view &)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Support/raw_ostream.h(260): note: or       'llvm::raw_ostream &llvm::raw_ostream::operator <<(const std::string &)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Support/raw_ostream.h(253): note: or       'llvm::raw_ostream &llvm::raw_ostream::operator <<(const char *)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Support/raw_ostream.h(224): note: or       'llvm::raw_ostream &llvm::raw_ostream::operator <<(llvm::StringRef)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Support/raw_ostream.h(217): note: or       'llvm::raw_ostream &llvm::raw_ostream::operator <<(signed char)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Support/raw_ostream.h(210): note: or       'llvm::raw_ostream &llvm::raw_ostream::operator <<(unsigned char)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Support/raw_ostream.h(203): note: or       'llvm::raw_ostream &llvm::raw_ostream::operator <<(char)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Support/KnownBits.h(532): note: or       'llvm::raw_ostream &llvm::operator <<(llvm::raw_ostream &,const llvm::KnownBits &)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Analysis/ScalarEvolution.h(251): note: or       'llvm::raw_ostream &llvm::operator <<(llvm::raw_ostream &,const llvm::SCEVPredicate &)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Analysis/ScalarEvolution.h(196): note: or       'llvm::raw_ostream &llvm::operator <<(llvm::raw_ostream &,const llvm::SCEV &)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Analysis/AliasAnalysis.h(145): note: or       'llvm::raw_ostream &llvm::operator <<(llvm::raw_ostream &,llvm::AliasResult)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/Analysis/MemoryLocation.h(213): note: or       'llvm::raw_ostream &llvm::operator <<(llvm::raw_ostream &,llvm::LocationSize)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/IR/Module.h(1075): note: or       'llvm::raw_ostream &llvm::operator <<(llvm::raw_ostream &,const llvm::Module &)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/IR/Comdat.h(71): note: or       'llvm::raw_ostream &llvm::operator <<(llvm::raw_ostream &,const llvm::Comdat &)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/IR/ConstantRange.h(603): note: or       'llvm::raw_ostream &llvm::operator <<(llvm::raw_ostream &,const llvm::ConstantRange &)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/ADT/APFloat.h(1600): note: or       'llvm::raw_ostream &llvm::operator <<(llvm::raw_ostream &,const llvm::APFloat &)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/ADT/FloatingPointMode.h(280): note: or       'llvm::raw_ostream &llvm::operator <<(llvm::raw_ostream &,llvm::FPClassTest)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/ADT/FloatingPointMode.h(183): note: or       'llvm::raw_ostream &llvm::operator <<(llvm::raw_ostream &,llvm::DenormalMode)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/ADT/FloatingPointMode.h(63): note: or       'llvm::raw_ostream &llvm::operator <<(llvm::raw_ostream &,llvm::RoundingMode)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/ADT/APInt.h(2152): note: or       'llvm::raw_ostream &llvm::operator <<(llvm::raw_ostream &,const llvm::APInt &)'
C:\buildbot\as-builder-3\llvm-clang-x86_64-win-fast\llvm-project\llvm\include\llvm/IR/InstrTypes.h(1043): note: or       'llvm::raw_ostream &llvm::operator <<(llvm::raw_ostream &,llvm::CmpInst::Predicate)'

vporpo added a commit that referenced this pull request Oct 25, 2024
winner245 pushed a commit to winner245/llvm-project that referenced this pull request Oct 26, 2024
winner245 pushed a commit to winner245/llvm-project that referenced this pull request Oct 26, 2024
winner245 pushed a commit to winner245/llvm-project that referenced this pull request Oct 26, 2024
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
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.

4 participants