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

[SLP][REVEC] Fix getStoreMinimumVF only accept scalar types. #132181

Merged
merged 2 commits into from
Mar 20, 2025

Conversation

HanKuanChen
Copy link
Contributor

Fix "Element type of a VectorType must " "be an integer, floating point,
or " "pointer type.".

Fix "Element type of a VectorType must " "be an integer, floating point,
or " "pointer type.".
@llvmbot
Copy link
Member

llvmbot commented Mar 20, 2025

@llvm/pr-subscribers-vectorizers

@llvm/pr-subscribers-llvm-transforms

Author: Han-Kuan Chen (HanKuanChen)

Changes

Fix "Element type of a VectorType must " "be an integer, floating point,
or " "pointer type.".


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp (+12-4)
  • (added) llvm/test/Transforms/SLPVectorizer/X86/revec-getStoreMinimumVF.ll (+17)
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index a2200f283168d..ad6e0a838cc1b 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -19697,10 +19697,18 @@ bool SLPVectorizerPass::vectorizeStores(
       Type *ValueTy = StoreTy;
       if (auto *Trunc = dyn_cast<TruncInst>(Store->getValueOperand()))
         ValueTy = Trunc->getSrcTy();
-      unsigned MinVF = std::max<unsigned>(
-          2, PowerOf2Ceil(TTI->getStoreMinimumVF(
-                 R.getMinVF(DL->getTypeStoreSizeInBits(StoreTy)), StoreTy,
-                 ValueTy)));
+      // When REVEC is enabled, StoreTy and ValueTy may be FixedVectorType. But
+      // getStoreMinimumVF only support scalar type as arguments. As a result,
+      // we need to use the element type of StoreTy and ValueTy to retrieve the
+      // VF and then transform it back.
+      // Remember: VF is defined as the number we want to vectorize, not the
+      // number of elements in the final vector.
+      Type *StoreScalarTy = StoreTy->getScalarType();
+      unsigned MinVF = PowerOf2Ceil(TTI->getStoreMinimumVF(
+          R.getMinVF(DL->getTypeStoreSizeInBits(StoreScalarTy)), StoreScalarTy,
+          ValueTy->getScalarType()));
+      MinVF /= getNumElements(StoreTy);
+      MinVF = std::max<unsigned>(2, MinVF);
 
       if (MaxVF < MinVF) {
         LLVM_DEBUG(dbgs() << "SLP: Vectorization infeasible as MaxVF (" << MaxVF
diff --git a/llvm/test/Transforms/SLPVectorizer/X86/revec-getStoreMinimumVF.ll b/llvm/test/Transforms/SLPVectorizer/X86/revec-getStoreMinimumVF.ll
new file mode 100644
index 0000000000000..3aea112e9edfe
--- /dev/null
+++ b/llvm/test/Transforms/SLPVectorizer/X86/revec-getStoreMinimumVF.ll
@@ -0,0 +1,17 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -mtriple=x86_64-unknown-linux-gnu -passes=slp-vectorizer -S -slp-revec %s | FileCheck %s
+
+define void @test() {
+; CHECK-LABEL: @test(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TMP0:%.*]] = call <8 x i8> @llvm.vector.insert.v8i8.v4i8(<8 x i8> poison, <4 x i8> zeroinitializer, i64 0)
+; CHECK-NEXT:    [[TMP1:%.*]] = call <8 x i8> @llvm.vector.insert.v8i8.v4i8(<8 x i8> [[TMP0]], <4 x i8> zeroinitializer, i64 4)
+; CHECK-NEXT:    store <8 x i8> [[TMP1]], ptr null, align 1
+; CHECK-NEXT:    ret void
+;
+entry:
+  %0 = getelementptr i8, ptr null, i64 4
+  store <4 x i8> zeroinitializer, ptr null, align 1
+  store <4 x i8> zeroinitializer, ptr %0, align 1
+  ret void
+}

@HanKuanChen HanKuanChen merged commit 73558dc into llvm:main Mar 20, 2025
12 of 14 checks passed
@HanKuanChen HanKuanChen deleted the slp-revec-getStoreMinimumVF branch March 20, 2025 13:04
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.

3 participants