[llvm] 1540f77 - Reapply "[SandboxVec][Legality] Reject non-instructions (#113190)"

[Vasileios Porpodas via llvm-commits]

Author: Vasileios Porpodas
Date: 2024-10-25T12:55:58-07:00
New Revision: 1540f772c793b3a29ae5d57e99456ec5d7ef4b39

URL: https://github.com/llvm/llvm-project/commit/1540f772c793b3a29ae5d57e99456ec5d7ef4b39
DIFF: https://github.com/llvm/llvm-project/commit/1540f772c793b3a29ae5d57e99456ec5d7ef4b39.diff

LOG: Reapply "[SandboxVec][Legality] Reject non-instructions (#113190)"

This reverts commit eb9f4756bc3daaa4b19f4f46521dc05180814de4.

Added: 
    

Modified: 
    llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Legality.h
    llvm/lib/Transforms/Vectorize/SandboxVectorizer/Legality.cpp
    llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
    llvm/unittests/Transforms/Vectorize/SandboxVectorizer/LegalityTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Legality.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Legality.h
index bcfafd75d4caaf..d4b0b54375b026 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,
 };
@@ -46,6 +47,8 @@ struct ToStr {
 
   static const char *getVecReason(ResultReason Reason) {
     switch (Reason) {
+    case ResultReason::NotInstructions:
+      return "NotInstructions";
     case ResultReason::DiffOpcodes:
       return "DiffOpcodes";
     case ResultReason::DiffTypes:
@@ -67,6 +70,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; }
@@ -90,6 +97,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);
@@ -138,7 +146,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..e4546c2f98113e 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,21 @@ LegalityAnalysis::notVectorizableBasedOnOpcodesAndTypes(
   return std::nullopt;
 }
 
-LegalityResult &LegalityAnalysis::canVectorize(ArrayRef<Value *> Bndl) {
+#ifndef NDEBUG
+static void dumpBndl(ArrayRef<Value *> Bndl) {
+  for (auto *V : Bndl)
+    dbgs() << *V << "\n";
+}
+#endif // NDEBUG
+
+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"));