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

[VectorCombine] Add a mask for SK_Broadcast shuffle costing #85808

Merged
merged 1 commit into from
Mar 19, 2024

Conversation

preames
Copy link
Collaborator

@preames preames commented Mar 19, 2024

This is part of a series of small patches to compute shuffle masks for the couple of cases where we call getShuffleCost without one. My goal is to add an invariant that all calls to getShuffleCost for fixed length vectors have a mask.

Note that this code appears to be reachable with scalable vectors, and thus we have to only pass a non-empty mask when the number of elements is precisely known.

This is part of a series of small patches to compute shuffle masks
for the couple of cases where we call getShuffleCost without one. My
goal is to add an invariant that all calls to getShuffleCost for fixed
length vectors have a mask.

Note that this code appears to be reachable with scalable vectors,
and thus we have to only pass a non-empty mask when the number of
elements is precisely known.
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 19, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Philip Reames (preames)

Changes

This is part of a series of small patches to compute shuffle masks for the couple of cases where we call getShuffleCost without one. My goal is to add an invariant that all calls to getShuffleCost for fixed length vectors have a mask.

Note that this code appears to be reachable with scalable vectors, and thus we have to only pass a non-empty mask when the number of elements is precisely known.


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

1 Files Affected:

  • (modified) llvm/lib/Transforms/Vectorize/VectorCombine.cpp (+4-1)
diff --git a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
index dc669e314a0d9a..85c8d3996bba51 100644
--- a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
+++ b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
@@ -786,9 +786,12 @@ bool VectorCombine::scalarizeVPIntrinsic(Instruction &I) {
   // intrinsic
   VectorType *VecTy = cast<VectorType>(VPI.getType());
   TTI::TargetCostKind CostKind = TTI::TCK_RecipThroughput;
+  SmallVector<int> Mask;
+  if (auto *FVTy = dyn_cast<FixedVectorType>(VecTy))
+    Mask.resize(FVTy->getNumElements(), 0);
   InstructionCost SplatCost =
       TTI.getVectorInstrCost(Instruction::InsertElement, VecTy, CostKind, 0) +
-      TTI.getShuffleCost(TargetTransformInfo::SK_Broadcast, VecTy);
+      TTI.getShuffleCost(TargetTransformInfo::SK_Broadcast, VecTy, Mask);
 
   // Calculate the cost of the VP Intrinsic
   SmallVector<Type *, 4> Args;

Copy link
Member

@alexey-bataev alexey-bataev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG

@preames preames merged commit 0081ec1 into llvm:main Mar 19, 2024
6 of 7 checks passed
@preames preames deleted the pr-vector-combine-broadcast-mask branch March 19, 2024 15:57
chencha3 pushed a commit to chencha3/llvm-project that referenced this pull request Mar 23, 2024
This is part of a series of small patches to compute shuffle masks for
the couple of cases where we call getShuffleCost without one. My goal is
to add an invariant that all calls to getShuffleCost for fixed length
vectors have a mask.

Note that this code appears to be reachable with scalable vectors, and
thus we have to only pass a non-empty mask when the number of elements
is precisely known.
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.

None yet

3 participants