-
Notifications
You must be signed in to change notification settings - Fork 14.7k
[VPlan] Materialize VF and VFxUF using VPInstructions. #152879
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
59322c5
3367a1f
e26258b
929db2e
4aa2925
f03fdfb
1cc3267
a404171
dab0d23
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 | ||||
---|---|---|---|---|---|---|
|
@@ -276,6 +276,20 @@ class VPBuilder { | |||||
return tryInsertInstruction(new VPPhi(IncomingValues, DL, Name)); | ||||||
} | ||||||
|
||||||
VPValue *createElementCount(Type *Ty, ElementCount EC) { | ||||||
VPlan &Plan = *getInsertBlock()->getPlan(); | ||||||
VPValue *RuntimeEC = | ||||||
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
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. I left this as RuntimeEC, because we use this for VFxUF as well, where the ElementCount passed in is the original VF multiplied by UF. |
||||||
Plan.getOrAddLiveIn(ConstantInt::get(Ty, EC.getKnownMinValue())); | ||||||
if (EC.isScalable()) { | ||||||
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. Early exit simpler complementary case first? 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. updated, thanks |
||||||
VPValue *VScale = createNaryOp(VPInstruction::VScale, {}, Ty); | ||||||
RuntimeEC = EC.getKnownMinValue() == 1 | ||||||
? VScale | ||||||
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. Early return VScale? 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. This is now directly returned at the end, I left the ternary operator there for now. |
||||||
: createOverflowingOp(Instruction::Mul, | ||||||
{VScale, RuntimeEC}, {true, 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. Comment constant parameters? 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. Added, thanks |
||||||
} | ||||||
return RuntimeEC; | ||||||
} | ||||||
|
||||||
/// Convert the input value \p Current to the corresponding value of an | ||||||
/// induction with \p Start and \p Step values, using \p Start + \p Current * | ||||||
/// \p Step. | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3336,6 +3336,42 @@ void VPlanTransforms::materializeVectorTripCount(VPlan &Plan, | |
VectorTC.replaceAllUsesWith(Res); | ||
} | ||
|
||
void VPlanTransforms::materializeVFAndVFxUF(VPlan &Plan, VPBasicBlock *VectorPH, | ||
ElementCount VFEC) { | ||
VPBuilder Builder(VectorPH, VectorPH->begin()); | ||
Type *TCTy = VPTypeAnalysis(Plan).inferScalarType(Plan.getTripCount()); | ||
VPValue &VF = Plan.getVF(); | ||
VPValue &VFxUF = Plan.getVFxUF(); | ||
Comment on lines
+3343
to
+3344
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. So Plan's getVF() and getVFxUF() become obsolete from this point? They will remain use-less, no longer retrieving the relevant values. Worth noting, or hooking them to their replacements? 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, for now, there are no users of getVF and getVFxUF at after this point. It would be good to mark them as such, but I'm not sure what the best way would be. Another alternative would be manage them as actual users. They should also be region-specific. I'll prepare some follow-ups for that. |
||
// Note that after the transform, Plan.getVF and Plan.getVFxUF should not be | ||
// used. | ||
// TODO: Assert that they aren't used. | ||
|
||
// If there are no users of the runtime VF, compute VFxUF by constant folding | ||
// the multiplication of VF and UF. | ||
if (VF.getNumUsers() == 0) { | ||
VPValue *RuntimeVFxUF = | ||
Builder.createElementCount(TCTy, VFEC * Plan.getUF()); | ||
VFxUF.replaceAllUsesWith(RuntimeVFxUF); | ||
return; | ||
} | ||
|
||
// For users of the runtime VF, compute it as VF * vscale, and VFxUF as (VF * | ||
// vscale) * UF. | ||
VPValue *RuntimeVF = Builder.createElementCount(TCTy, VFEC); | ||
if (any_of(VF.users(), [&VF](VPUser *U) { return !U->usesScalars(&VF); })) { | ||
VPValue *BC = Builder.createNaryOp(VPInstruction::Broadcast, RuntimeVF); | ||
VF.replaceUsesWithIf( | ||
BC, [&VF](VPUser &U, unsigned) { return !U.usesScalars(&VF); }); | ||
} | ||
VF.replaceAllUsesWith(RuntimeVF); | ||
|
||
VPValue *UF = Plan.getOrAddLiveIn(ConstantInt::get(TCTy, Plan.getUF())); | ||
VPValue *MulByUF = Plan.getUF() == 1 ? RuntimeVF | ||
: Builder.createNaryOp(Instruction::Mul, | ||
{RuntimeVF, UF}); | ||
VFxUF.replaceAllUsesWith(MulByUF); | ||
} | ||
|
||
/// Returns true if \p V is VPWidenLoadRecipe or VPInterleaveRecipe that can be | ||
/// converted to a narrower recipe. \p V is used by a wide recipe that feeds a | ||
/// store interleave group at index \p Idx, \p WideMember0 is the recipe feeding | ||
|
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.
Sounds a bit odd to createElementCount given an ElementCount. There's probably a better name than "RuntimeVF", but that's already in use elsewhere, in any case should be clearly defined.
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.
IRBuilder has an identical function also called
IRBuilderBase::CreateElementCount
, and SelectionDAG too withSelectionDAG::getElementCount
. I think we should be consistent here.It also doesn't necessarily need to generate a VF. An ElementCount could be e.g. VFxUF or another arbitrary quantity.
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 left the name aligned with the IR Builder for now.