-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[VPlan] Don't rely on region check in isUniformAfterVectorization. #137883
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 4 commits
1d63029
390b349
de47c85
02316d2
b42be1b
505820e
659e0cf
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 |
---|---|---|
|
@@ -39,19 +39,43 @@ const SCEV *getSCEVExprForVPValue(VPValue *V, ScalarEvolution &SE); | |
|
||
/// Returns true if \p VPV is uniform after vectorization. | ||
inline bool isUniformAfterVectorization(const VPValue *VPV) { | ||
// A value defined outside the vector region must be uniform after | ||
// vectorization inside a vector region. | ||
if (VPV->isDefinedOutsideLoopRegions()) | ||
auto PreservesUniformity = [](unsigned Opcode) -> bool { | ||
if (Instruction::isBinaryOp(Opcode) || Instruction::isCast(Opcode)) | ||
return true; | ||
switch (Opcode) { | ||
case Instruction::GetElementPtr: | ||
case Instruction::ICmp: | ||
case Instruction::FCmp: | ||
case VPInstruction::Broadcast: | ||
case VPInstruction::PtrAdd: | ||
return true; | ||
default: | ||
return false; | ||
} | ||
}; | ||
|
||
// A live-in must be uniform across the scope of VPlan. | ||
if (VPV->isLiveIn()) | ||
return true; | ||
if (auto *Rep = dyn_cast<VPReplicateRecipe>(VPV)) | ||
return Rep->isUniform(); | ||
|
||
if (auto *Rep = dyn_cast<VPReplicateRecipe>(VPV)) { | ||
const VPRegionBlock *RegionOfR = Rep->getParent()->getParent(); | ||
if (RegionOfR && RegionOfR->isReplicator()) | ||
return false; | ||
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. Maybe worth a comment - it's ok functionally to sink uniform/after-vectorization recipes into replicate regions, we avoid doing so due to cost considerations. Here an early exit condition is introduced before checking operands recursively - another cost rather than functional decision - could do w/o the replicate region check? (Topic of patch is to avoid relying on regions.) Note that prior to introducing replicate regions, the recipes they are to include can (and should?) be identified by isPredicated(). 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. Yep, we could consider them uniform, the main issue is that this is also used when executing replicate recipes to see if we can simply retrieve the first lane; but that's not possible for replicate regions, as the uniform value created for lane 0 won't dominate the user when executing the region for lane 1 and so on. added a comment |
||
return Rep->isUniform() || | ||
(PreservesUniformity(Rep->getOpcode()) && | ||
all_of(Rep->operands(), isUniformAfterVectorization)); | ||
} | ||
if (isa<VPWidenGEPRecipe, VPDerivedIVRecipe, VPBlendRecipe>(VPV)) | ||
return all_of(VPV->getDefiningRecipe()->operands(), | ||
isUniformAfterVectorization); | ||
if (auto *WidenR = dyn_cast<VPWidenRecipe>(VPV)) { | ||
return PreservesUniformity(WidenR->getOpcode()) && | ||
all_of(WidenR->operands(), isUniformAfterVectorization); | ||
} | ||
if (auto *VPI = dyn_cast<VPInstruction>(VPV)) | ||
return VPI->isSingleScalar() || VPI->isVectorToScalar() || | ||
((Instruction::isBinaryOp(VPI->getOpcode()) || | ||
VPI->getOpcode() == VPInstruction::PtrAdd) && | ||
(PreservesUniformity(VPI->getOpcode()) && | ||
all_of(VPI->operands(), isUniformAfterVectorization)); | ||
|
||
// VPExpandSCEVRecipes must be placed in the entry and are alway uniform. | ||
|
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.
Independent: documentation should be (more) meaningful. Name should be more accurate.
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.
Will check separately