-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[LV] Vectorize conditional scalar assignments #158088
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
base: main
Are you sure you want to change the base?
Changes from 23 commits
e08017c
ee66898
da74e57
016adab
2c3fb8f
f1b8bcd
5e75d92
2cfbbfb
436fd2f
74f0351
efd9d17
72a1b8c
9fe68d2
9cbdf21
bb8106f
5d1be36
3708486
2b43298
87b837b
570f0e3
5df3db6
962a23c
12a7cd8
c7b50ac
d787a5c
8cf2168
c8159ce
42ea80d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -58,6 +58,8 @@ bool RecurrenceDescriptor::isIntegerRecurrenceKind(RecurKind Kind) { | |||||
| case RecurKind::FindFirstIVUMin: | ||||||
| case RecurKind::FindLastIVSMax: | ||||||
| case RecurKind::FindLastIVUMax: | ||||||
| // TODO: Make type-agnostic. | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this is not type-agnostic, should this be reflected in the name of the recurrence kind?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It isn't for the other FindFirst/FindLast (though that might be inferred by U/S Min/Max) or AnyOf. I think it's just the fp-based reduction types that are prefixed with an extra I did experiment with treating FindLast separately in |
||||||
| case RecurKind::FindLast: | ||||||
| return true; | ||||||
| } | ||||||
| return false; | ||||||
|
|
@@ -721,9 +723,15 @@ RecurrenceDescriptor::isAnyOfPattern(Loop *Loop, PHINode *OrigPhi, | |||||
| // if (src[i] > 3) | ||||||
| // r = i; | ||||||
| // } | ||||||
| // or like this: | ||||||
| // int r = 0; | ||||||
| // for (int i = 0; i < n; i++) { | ||||||
| // if (a[i] > 3) | ||||||
| // r = a[i]; | ||||||
sdesmalen-arm marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| // } | ||||||
| // The reduction value (r) is derived from either the values of an induction | ||||||
| // variable (i) sequence, or from the start value (0). The LLVM IR generated for | ||||||
| // such loops would be as follows: | ||||||
| // variable (i) sequence, an arbitrary value (a[i]), or from the start value | ||||||
|
||||||
| // variable (i) sequence, an arbitrary value (a[i]), or from the start value | |
| // variable (i) sequence, an arbitrary value (src[i]), or from the start value |
(or the more generic term if updated above)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this isn't your writing, but this is very difficult to follow..
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be good to keep term (currently a[i]) in sync with what the example uses
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
sdesmalen-arm marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -4302,11 +4302,15 @@ bool LoopVectorizationPlanner::isCandidateForEpilogueVectorization( | |||||
| ElementCount VF) const { | ||||||
| // Cross iteration phis such as fixed-order recurrences and FMaxNum/FMinNum | ||||||
| // reductions need special handling and are currently unsupported. | ||||||
| // FindLast reductions also require special handling for the synthesized | ||||||
| // mask PHI. | ||||||
| if (any_of(OrigLoop->getHeader()->phis(), [&](PHINode &Phi) { | ||||||
| if (!Legal->isReductionVariable(&Phi)) | ||||||
| return Legal->isFixedOrderRecurrence(&Phi); | ||||||
| return RecurrenceDescriptor::isFPMinMaxNumRecurrenceKind( | ||||||
| Legal->getRecurrenceDescriptor(&Phi).getRecurrenceKind()); | ||||||
| RecurKind Kind = | ||||||
| Legal->getRecurrenceDescriptor(&Phi).getRecurrenceKind(); | ||||||
| return RecurrenceDescriptor::isFindLastRecurrenceKind(Kind) || | ||||||
| RecurrenceDescriptor::isFPMinMaxNumRecurrenceKind(Kind); | ||||||
| })) | ||||||
| return false; | ||||||
|
|
||||||
|
|
@@ -4612,6 +4616,14 @@ LoopVectorizationPlanner::selectInterleaveCount(VPlan &Plan, ElementCount VF, | |||||
| any_of(Plan.getVectorLoopRegion()->getEntryBasicBlock()->phis(), | ||||||
| IsaPred<VPReductionPHIRecipe>); | ||||||
|
|
||||||
| // FIXME: implement interleaving for FindLast transform correctly. | ||||||
| if (any_of(make_second_range(Legal->getReductionVars()), | ||||||
| [](const RecurrenceDescriptor &RdxDesc) { | ||||||
| return RecurrenceDescriptor::isFindLastRecurrenceKind( | ||||||
| RdxDesc.getRecurrenceKind()); | ||||||
| })) | ||||||
| return 1; | ||||||
|
|
||||||
| // If we did not calculate the cost for VF (because the user selected the VF) | ||||||
| // then we calculate the cost of VF here. | ||||||
| if (LoopCost == 0) { | ||||||
|
|
@@ -8586,6 +8598,11 @@ VPlanPtr LoopVectorizationPlanner::tryToBuildVPlanWithVPRecipes( | |||||
| *Plan)) | ||||||
| return nullptr; | ||||||
|
|
||||||
| // Create whole-vector selects for find-last recurrences. | ||||||
| if (!VPlanTransforms::runPass(VPlanTransforms::handleFindLastReductions, | ||||||
| *Plan, RecipeBuilder)) | ||||||
| return nullptr; | ||||||
|
|
||||||
| // Transform recipes to abstract recipes if it is legal and beneficial and | ||||||
| // clamp the range for better cost estimation. | ||||||
| // TODO: Enable following transform when the EVL-version of extended-reduction | ||||||
|
|
@@ -8707,10 +8724,11 @@ void LoopVectorizationPlanner::adjustRecipesForReductions( | |||||
| continue; | ||||||
|
|
||||||
| RecurKind Kind = PhiR->getRecurrenceKind(); | ||||||
| assert( | ||||||
| !RecurrenceDescriptor::isAnyOfRecurrenceKind(Kind) && | ||||||
| !RecurrenceDescriptor::isFindIVRecurrenceKind(Kind) && | ||||||
| "AnyOf and FindIV reductions are not allowed for in-loop reductions"); | ||||||
| assert(!RecurrenceDescriptor::isFindLastRecurrenceKind(Kind) && | ||||||
| !RecurrenceDescriptor::isAnyOfRecurrenceKind(Kind) && | ||||||
| !RecurrenceDescriptor::isFindIVRecurrenceKind(Kind) && | ||||||
| "AnyOf, FindIV, and FindLast reductions are not allowed for in-loop " | ||||||
| "reductions"); | ||||||
|
|
||||||
| bool IsFPRecurrence = | ||||||
| RecurrenceDescriptor::isFloatingPointRecurrenceKind(Kind); | ||||||
|
|
@@ -9017,7 +9035,8 @@ void LoopVectorizationPlanner::adjustRecipesForReductions( | |||||
| RecurKind RK = RdxDesc.getRecurrenceKind(); | ||||||
| if ((!RecurrenceDescriptor::isAnyOfRecurrenceKind(RK) && | ||||||
| !RecurrenceDescriptor::isFindIVRecurrenceKind(RK) && | ||||||
| !RecurrenceDescriptor::isMinMaxRecurrenceKind(RK))) { | ||||||
| !RecurrenceDescriptor::isMinMaxRecurrenceKind(RK) && | ||||||
| !RecurrenceDescriptor::isFindLastRecurrenceKind(RK))) { | ||||||
| VPBuilder PHBuilder(Plan->getVectorPreheader()); | ||||||
| VPValue *Iden = Plan->getOrAddLiveIn( | ||||||
| getRecurrenceIdentity(RK, PhiTy, RdxDesc.getFastMathFlags())); | ||||||
|
|
@@ -10148,6 +10167,21 @@ bool LoopVectorizePass::processLoop(Loop *L) { | |||||
| // Override IC if user provided an interleave count. | ||||||
| IC = UserIC > 0 ? UserIC : IC; | ||||||
|
|
||||||
| // FIXME: Enable interleaving for last_active reductions. | ||||||
|
||||||
| // FIXME: Enable interleaving for last_active reductions. | |
| // FIXME: Enable interleaving for FindLast reductions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| << "to conditional scalar assignments.\n"); | |
| << "to FindLast reduction.\n"); |
for consistency with the reduction naming
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "ConditionalAssignmentPreventsScalarInterleaving", | |
| "FindLastPreventsScalarInterleaving", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think at least loop-unrolling also needs to be thought about the new kind, seeing crashes when building the test suite currently. I think to reproduce you can just add a loop with CAS to https://github.com/llvm/llvm-project/blob/main/llvm/test/Transforms/LoopUnroll/partial-unroll-reductions.ll.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done. I think I left it alone originally since I made FindLast a generic RecurKind that could handle int, float, and pointer types (and it therefore didn't appear in the isIntegerRecurrenceKind list.) I figured that could be a follow-up PR (and could potentially convert AnyOf at the same time).