[SandboxVec][NFC] Avoid temporary vectors.

[vporpo]

This patch fixes some of the TODOs where we were using a temporary vector as a means to convert a container of Value * to a container of Instruction * and vice versa.

We introduce a helper VecUtils function that allows us to convert ArrayRefs of one type to the other.

[llvmbot]

@llvm/pr-subscribers-vectorizers

@llvm/pr-subscribers-llvm-transforms

Author: vporpo (vporpo)

Changes

This patch fixes some of the TODOs where we were using a temporary vector as a means to convert a container of Value * to a container of Instruction * and vice versa.

We introduce a helper VecUtils function that allows us to convert ArrayRefs of one type to the other.

---

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

5 Files Affected:

• (modified) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/VecUtils.h (+10)
• (modified) llvm/lib/Transforms/Vectorize/SandboxVectorizer/Legality.cpp (+1-6)
• (modified) llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp (+1-2)
• (modified) llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/LoadStoreVec.cpp (+2-6)
• (modified) llvm/unittests/Transforms/Vectorize/SandboxVectorizer/VecUtilsTest.cpp (+31)

diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/VecUtils.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/VecUtils.h
index 515097e4f9924..35adb8c0267b4 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/VecUtils.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/VecUtils.h
@@ -384,6 +384,16 @@ class VecUtils {
     return make_range(Begin, End);
   }
 
+  /// Helps convert things like `ArrayRef<Instruction *>` to `ArrayRef<Value *>`
+  /// and vice versa. For example, given an `ArrayRef<Instruction *> Instrs`, we
+  /// can get an ArrayRef of values:
+  ///   `ArrayRef<Value *> Vals = VecUtils::toArrayRef<Value *>(Instrs);`
+  template <typename ToElmT, typename FromArrayRefT>
+  static ArrayRef<ToElmT> toArrayRef(FromArrayRefT From) {
+    return ArrayRef<ToElmT>(reinterpret_cast<const ToElmT *>(From.data()),
+                            From.size());
+  }
+
 #ifndef NDEBUG
   /// Helper dump function for debugging.
   LLVM_DUMP_METHOD static void dump(ArrayRef<Value *> Bndl);
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Legality.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Legality.cpp
index 9976aaf845705..d2d5d94fd397a 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Legality.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Legality.cpp
@@ -241,12 +241,7 @@ const LegalityResult &LegalityAnalysis::canVectorize(ArrayRef<Value *> Bndl,
     return createLegalityResult<Pack>(*ReasonOpt);
 
   if (!SkipScheduling) {
-    // TODO: Try to remove the IBndl vector.
-    SmallVector<Instruction *, 8> IBndl;
-    IBndl.reserve(Bndl.size());
-    for (auto *V : Bndl)
-      IBndl.push_back(cast<Instruction>(V));
-    if (!Sched.trySchedule(IBndl))
+    if (!Sched.trySchedule(VecUtils::toArrayRef<Instruction *>(Bndl)))
       return createLegalityResult<Pack>(ResultReason::CantSchedule);
   }
 
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
index 6bf257fcf8b1d..4823109991fa4 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
@@ -542,8 +542,7 @@ bool BottomUpVec::runOnRegion(Region &Rgn, const Analyses &A) {
                             F.getParent()->getDataLayout(), F.getContext(),
                             *IMaps);
 
-  // TODO: Refactor to remove the unnecessary copy to SeedSliceVals.
-  SmallVector<Value *> SeedSliceVals(SeedSlice.begin(), SeedSlice.end());
+  ArrayRef<Value *> SeedSliceVals = VecUtils::toArrayRef<Value *>(SeedSlice);
   // Try to vectorize starting from the seed slice. The returned value
   // is true if we found vectorizable code and generated some vector
   // code for it. It does not mean that the code is profitable.
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/LoadStoreVec.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/LoadStoreVec.cpp
index 0436f963597a4..dd9ad7f0daeb8 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/LoadStoreVec.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/LoadStoreVec.cpp
@@ -111,12 +111,8 @@ bool LoadStoreVec::runOnRegion(Region &Rgn, const Analyses &A) {
 
   Value *VecOp = nullptr;
   if (AllLoads) {
-    // TODO: Try to avoid the extra copy to an instruction vector.
-    SmallVector<Instruction *, 8> Loads;
-    Loads.reserve(Operands.size());
-    for (Value *Op : Operands)
-      Loads.push_back(cast<Instruction>(Op));
-
+    ArrayRef<Instruction *> Loads =
+        VecUtils::toArrayRef<Instruction *>(Operands);
     bool Consecutive = VecUtils::areConsecutive<LoadInst, Instruction>(
         Loads, A.getScalarEvolution(), *DL);
     if (!Consecutive)
diff --git a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/VecUtilsTest.cpp b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/VecUtilsTest.cpp
index 9ade4b0638846..f13ca4795f00d 100644
--- a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/VecUtilsTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/VecUtilsTest.cpp
@@ -791,3 +791,34 @@ define void @foo(i32 %s0, <4 x i32> %v0, i32 %s1, <2 x i32> %v1, <3 x i32> %v2,
   EXPECT_EQ(Elms, Bndl);
   EXPECT_THAT(Lanes, testing::ElementsAre(0, 1, 5, 6, 8, 11));
 }
+
+TEST_F(VecUtilsTest, ToArrayRef) {
+  parseIR(R"IR(
+define void @foo(i8 %v0, i8 %v1, i8 %v2) {
+bb0:
+  %add0 = add i8 %v0, %v0
+  %add1 = add i8 %v1, %v1
+  %add2 = add i8 %v2, %v2
+  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();
+  sandboxir::Instruction *I0 = &*It++;
+  sandboxir::Instruction *I1 = &*It++;
+  sandboxir::Instruction *I2 = &*It++;
+  SmallVector<sandboxir::Instruction *> Instrs({I0, I1, I2});
+
+  // Check ArrayRef<Instruction *> to ArrayRef<Value *>.
+  ArrayRef<sandboxir::Value *> Vals =
+      sandboxir::VecUtils::toArrayRef<sandboxir::Value *>(Instrs);
+  EXPECT_THAT(Vals, testing::ElementsAre(I0, I1, I2));
+
+  // Check ArrayRef<Value *> to ArrayRef<Instruction *>.
+  ArrayRef<sandboxir::Instruction *> NewInstrs =
+      sandboxir::VecUtils::toArrayRef<sandboxir::Instruction *>(Vals);
+  EXPECT_THAT(NewInstrs, testing::ElementsAre(I0, I1, I2));
+}