[VPlan] Remove ExtractLastLane for plans with scalar VFs. #171145
Merged
+18
−13
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ExtractLastLane is a no-op for scalar VFs. Update simplifyRecipe to remove them. This also requires adjusting the code in VPlanUnroll.cpp to split off handling of ExtractLastLane/ExtractPenultimateElement for scalar VFs, which now needs to match ExtractLastPart.
Member
|
@llvm/pr-subscribers-vectorizers @llvm/pr-subscribers-llvm-transforms Author: Florian Hahn (fhahn) ChangesExtractLastLane is a no-op for scalar VFs. Update simplifyRecipe to remove them. This also requires adjusting the code in VPlanUnroll.cpp to split off handling of ExtractLastLane/ExtractPenultimateElement for scalar VFs, which now needs to match ExtractLastPart. Full diff: https://github.com/llvm/llvm-project/pull/171145.diff 3 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index 320baeb454d46..4ad098d748568 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -1385,12 +1385,16 @@ static void simplifyRecipe(VPSingleDefRecipe *Def, VPTypeAnalysis &TypeInfo) {
return;
}
- // Look through ExtractLastLane (BuildVector ....).
- if (match(Def, m_ExtractLastLane(m_BuildVector()))) {
- auto *BuildVector = cast<VPInstruction>(Def->getOperand(0));
- Def->replaceAllUsesWith(
- BuildVector->getOperand(BuildVector->getNumOperands() - 1));
- return;
+ // Look through ExtractLastLane.
+ if (match(Def, m_ExtractLastLane(m_VPValue(A)))) {
+ if (match(A, m_BuildVector())) {
+ auto *BuildVector = cast<VPInstruction>(A);
+ Def->replaceAllUsesWith(
+ BuildVector->getOperand(BuildVector->getNumOperands() - 1));
+ return;
+ }
+ if (Plan->hasScalarVFOnly())
+ return Def->replaceAllUsesWith(A);
}
// Look through ExtractPenultimateElement (BuildVector ....).
diff --git a/llvm/lib/Transforms/Vectorize/VPlanUnroll.cpp b/llvm/lib/Transforms/Vectorize/VPlanUnroll.cpp
index 6fb706ea7d64b..7b4c524712d9a 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanUnroll.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanUnroll.cpp
@@ -371,10 +371,9 @@ void UnrollState::unrollBlock(VPBlockBase *VPB) {
continue;
}
- if (match(&R, m_ExtractLastLaneOfLastPart(m_VPValue(Op0))) ||
- match(&R, m_ExtractPenultimateElement(m_VPValue(Op0)))) {
- addUniformForAllParts(cast<VPSingleDefRecipe>(&R));
- if (Plan.hasScalarVFOnly()) {
+ if (Plan.hasScalarVFOnly()) {
+ if (match(&R, m_ExtractLastPart(m_VPValue(Op0))) ||
+ match(&R, m_ExtractPenultimateElement(m_VPValue(Op0)))) {
auto *I = cast<VPInstruction>(&R);
bool IsPenultimatePart =
I->getOpcode() == VPInstruction::ExtractPenultimateElement;
@@ -383,7 +382,10 @@ void UnrollState::unrollBlock(VPBlockBase *VPB) {
I->replaceAllUsesWith(getValueForPart(Op0, PartIdx));
continue;
}
- // For vector VF, always extract from the last part.
+ }
+ if (match(&R, m_ExtractLastLaneOfLastPart(m_VPValue(Op0))) ||
+ match(&R, m_ExtractPenultimateElement(m_VPValue(Op0)))) {
+ addUniformForAllParts(cast<VPSingleDefRecipe>(&R));
R.setOperand(0, getValueForPart(Op0, UF - 1));
continue;
}
diff --git a/llvm/test/Transforms/LoopVectorize/interleave-and-scalarize-only.ll b/llvm/test/Transforms/LoopVectorize/interleave-and-scalarize-only.ll
index bbd596a772c53..c77afa870e2c1 100644
--- a/llvm/test/Transforms/LoopVectorize/interleave-and-scalarize-only.ll
+++ b/llvm/test/Transforms/LoopVectorize/interleave-and-scalarize-only.ll
@@ -220,7 +220,6 @@ exit:
; DBG-EMPTY:
; DBG-NEXT: middle.block:
; DBG-NEXT: EMIT vp<[[RESUME_1_PART:%.+]]> = extract-last-part vp<[[SCALAR_STEPS]]>
-; DBG-NEXT: EMIT vp<[[RESUME_1:%.+]]> = extract-last-lane vp<[[RESUME_1_PART]]>
; DBG-NEXT: EMIT vp<[[CMP:%.+]]> = icmp eq vp<[[TC]]>, vp<[[VEC_TC]]>
; DBG-NEXT: EMIT branch-on-cond vp<[[CMP]]>
; DBG-NEXT: Successor(s): ir-bb<exit>, scalar.ph
@@ -230,7 +229,7 @@ exit:
; DBG-EMPTY:
; DBG-NEXT: scalar.ph:
; DBG-NEXT: EMIT-SCALAR vp<[[RESUME_IV:%.+]]> = phi [ vp<[[VTC]]>, middle.block ], [ ir<0>, ir-bb<entry> ]
-; DBG-NEXT: EMIT-SCALAR vp<[[RESUME_P:%.*]]> = phi [ vp<[[RESUME_1]]>, middle.block ], [ ir<0>, ir-bb<entry> ]
+; DBG-NEXT: EMIT-SCALAR vp<[[RESUME_P:%.*]]> = phi [ vp<[[RESUME_1_PART]]>, middle.block ], [ ir<0>, ir-bb<entry> ]
; DBG-NEXT: Successor(s): ir-bb<loop>
; DBG-EMPTY:
; DBG-NEXT: ir-bb<loop>:
|
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Dec 9, 2025
…(#171145) ExtractLastLane is a no-op for scalar VFs. Update simplifyRecipe to remove them. This also requires adjusting the code in VPlanUnroll.cpp to split off handling of ExtractLastLane/ExtractPenultimateElement for scalar VFs, which now needs to match ExtractLastPart. PR: llvm/llvm-project#171145
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/95/builds/19379 Here is the relevant piece of the build log for the reference |
ayalz
reviewed
Dec 11, 2025
Collaborator
ayalz
left a comment
ayalz
left a comment
There was a problem hiding this comment.
Couple of post-commits nits.
| return; | ||
| // Look through ExtractLastLane. | ||
| if (match(Def, m_ExtractLastLane(m_VPValue(A)))) { | ||
| if (match(A, m_BuildVector())) { |
Collaborator
There was a problem hiding this comment.
Such BuildVector operands were visited and processed earlier by the code below folding them into Broadcast if all their operands are equal. Does that folding hold for VF=1 too; if so should such redundant Broadcasts be eliminated too? Worth looking for extracts from broadcast too/instead?
Contributor
Author
There was a problem hiding this comment.
It looks like currently there are no extracts of broadcasts that could be folded in the tests. For VF=1, the broadcasts itself should be removed, possibly before hitting this pattern, but I might be missing something.
Comment on lines
+375
to
+376
| if (match(&R, m_ExtractLastPart(m_VPValue(Op0))) || | ||
| match(&R, m_ExtractPenultimateElement(m_VPValue(Op0)))) { |
Collaborator
There was a problem hiding this comment.
Would be simpler handle each match separately?
Contributor
Author
There was a problem hiding this comment.
I am not sure, the only difference is the index to extract from?
Collaborator
There was a problem hiding this comment.
Right, but folding that difference seems to complicate two trivial cases, the first which should hold for any VF, scalar or not:
if (match(&R, m_ExtractLastPart(m_VPValue(Op0)))) {
cast<VPInstruction>(&R)->replaceAllUsesWith(getValueForPart(Op0, UF - 1));
continue;
}
and the second which deals with scalar VF only:
if (Plan.hasScalarVFOnly() && match(&R, m_ExtractPenultimateElement(m_VPValue(Op0)))) {
// For scalar VF, use the penultimate scalar part w/o extracting a lane.
cast<VPInstruction>(&R)->replaceAllUsesWith(getValueForPart(Op0, UF - 2));
continue;
}
?
Are both missing addUniformForAllParts(cast<VPSingleDefRecipe>(&R));?
Can also hoist from below the auto *SingleDef = dyn_cast<VPSingleDefRecipe>(&R) to be used for the RAUW's here and addUniformForAllParts() elsewhere, instead of having to cast<>(&R) after various matches. Perhaps also handle the !SingleDef cases first, to freely access SingleDef afterwards.
| I->replaceAllUsesWith(getValueForPart(Op0, PartIdx)); | ||
| continue; | ||
| } | ||
| // For vector VF, always extract from the last part. |
Collaborator
There was a problem hiding this comment.
Worth retaining the comment, perhaps emphasizing that
// For vector VF, the penultimate element is always extracted from the last part.
fhahn
added a commit
that referenced
this pull request
Dec 12, 2025
Re-add and emphasize comment regarding extracting from the last part, as suggested post-commit in #171145.
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Dec 12, 2025
…t. (NFC) Re-add and emphasize comment regarding extracting from the last part, as suggested post-commit in llvm/llvm-project#171145.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ExtractLastLane is a no-op for scalar VFs. Update simplifyRecipe to remove them. This also requires adjusting the code in VPlanUnroll.cpp to split off handling of ExtractLastLane/ExtractPenultimateElement for scalar VFs, which now needs to match ExtractLastPart.