-
Notifications
You must be signed in to change notification settings - Fork 15k
[VPlan] Extract reverse mask from reverse accesses #155579
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?
Conversation
✅ With the latest revision this PR passed the C/C++ code formatter. |
@@ -1338,248 +1338,49 @@ for.end: ; preds = %for.inc | |||
;} | |||
|
|||
define void @foo6(ptr nocapture readonly %in, ptr nocapture %out, i32 %size, ptr nocapture readonly %trigger) local_unnamed_addr #0 { | |||
; AVX1-LABEL: @foo6( |
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.
// The legacy model have not computed the cost of reverse mask. | ||
if (CostCtx.CM.Legal->isMaskRequired(UI)) | ||
return true; |
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.
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.
@fhahn This could lead to overestimating the cost.
Multiple VPWidenMemoryRecipes may be able to share a single reverse mask, but if each VPWidenMemoryRecipe computes the cost of the reverse mask separately, the cost would be overestimated.
bool IsReverse = CostCtx.CM.getWideningDecision(UI, VF) == | ||
LoopVectorizationCostModel::CM_Widen_Reverse; |
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.
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.
@fhahn I think this is the most straightforward approach.
Currently, we could check whether the operand or user is a reverse operation, but in the future reverse operations might be simplified away, so relying on reverse operation is not a long-term approach.
The last option is to use the address for the check, since only reverse operations need to use VPVectorEndPointer.
What do you think?
Following #146525, separate the reverse mask from reverse access recipes.
At the same time, remove the unused member variable
Reverse
fromVPWidenMemoryRecipe
.This will help to reduce redundant reverse mask computations once VPlan-based CSE is supported.
Base on #146525