-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[LV] Explicitly disable in-loop reductions for AnyOf and FindIV. nfc #163541
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
Changes from all commits
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 | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1220,11 +1220,6 @@ unsigned RecurrenceDescriptor::getOpcode(RecurKind Kind) { | |||||||||||
| return Instruction::Add; | ||||||||||||
| case RecurKind::Mul: | ||||||||||||
| return Instruction::Mul; | ||||||||||||
| case RecurKind::AnyOf: | ||||||||||||
| case RecurKind::FindFirstIVSMin: | ||||||||||||
| case RecurKind::FindFirstIVUMin: | ||||||||||||
| case RecurKind::FindLastIVSMax: | ||||||||||||
| case RecurKind::FindLastIVUMax: | ||||||||||||
| case RecurKind::Or: | ||||||||||||
| return Instruction::Or; | ||||||||||||
| case RecurKind::And: | ||||||||||||
|
|
@@ -1248,6 +1243,13 @@ unsigned RecurrenceDescriptor::getOpcode(RecurKind Kind) { | |||||||||||
| case RecurKind::FMaximumNum: | ||||||||||||
| case RecurKind::FMinimumNum: | ||||||||||||
| return Instruction::FCmp; | ||||||||||||
| case RecurKind::AnyOf: | ||||||||||||
| case RecurKind::FindFirstIVSMin: | ||||||||||||
| case RecurKind::FindFirstIVUMin: | ||||||||||||
| case RecurKind::FindLastIVSMax: | ||||||||||||
| case RecurKind::FindLastIVUMax: | ||||||||||||
| // TODO: Set AnyOf and FindIV to Instruction::Select once in-loop reductions | ||||||||||||
|
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.
Boolean recurrence chain of AnyOf could indeed contain a Select as in Integer recurrence chain of FindIV indeed contains a Select: Suffice for now to leave behind a TODO to return a valid opcode once these kinds of recurrences support in-loop reduction. |
||||||||||||
| // are supported. | ||||||||||||
|
Comment on lines
+1251
to
+1252
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.
Suggested change
rather than claiming they are Unknown recurrence operations? Should the enclosing |
||||||||||||
| default: | ||||||||||||
| llvm_unreachable("Unknown recurrence operation"); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
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.
Could we still leave AnyOf to return
Orto avoid the changes inllvm/lib/Transforms/Vectorize/VPlanRecipes.cpp?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 we should maintain consistency in how we handle AnyOf and FindIV. In particular, AnyOf, whether it currently only supports integer types or will support floating-point types in the future, should consistently use Instruction::Or in ComputeAnyOfResult to fold each part.
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.
Ok sounds good to me.