-
Notifications
You must be signed in to change notification settings - Fork 15.5k
Revert [VPlan] Consolidate logic for narrowToSingleScalars #170720
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
Revert [VPlan] Consolidate logic for narrowToSingleScalars #170720
Conversation
This reverts commit 7b3ec51, as a crash was reported: https://llvm.godbolt.org/z/dK6ff5zvr -- this will give us time to investigate a re-land.
|
@llvm/pr-subscribers-llvm-transforms Author: Ramkumar Ramachandra (artagnon) ChangesThis reverts commit 7b3ec51, as a crash was reported: https://llvm.godbolt.org/z/dK6ff5zvr -- this will give us time to investigate a re-land. Full diff: https://github.com/llvm/llvm-project/pull/170720.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index b3ba38f6c630e..018c2d21bf46f 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -757,6 +757,31 @@ static void legalizeAndOptimizeInductions(VPlan &Plan) {
if (!PhiR)
continue;
+ // Try to narrow wide and replicating recipes to uniform recipes, based on
+ // VPlan analysis.
+ // TODO: Apply to all recipes in the future, to replace legacy uniformity
+ // analysis.
+ auto Users = collectUsersRecursively(PhiR);
+ for (VPUser *U : reverse(Users)) {
+ auto *Def = dyn_cast<VPRecipeWithIRFlags>(U);
+ auto *RepR = dyn_cast<VPReplicateRecipe>(U);
+ // Skip recipes that shouldn't be narrowed.
+ if (!Def || !isa<VPReplicateRecipe, VPWidenRecipe>(Def) ||
+ Def->getNumUsers() == 0 || !Def->getUnderlyingValue() ||
+ (RepR && (RepR->isSingleScalar() || RepR->isPredicated())))
+ continue;
+
+ // Skip recipes that may have other lanes than their first used.
+ if (!vputils::isSingleScalar(Def) && !vputils::onlyFirstLaneUsed(Def))
+ continue;
+
+ auto *Clone = new VPReplicateRecipe(Def->getUnderlyingInstr(),
+ Def->operands(), /*IsUniform*/ true,
+ /*Mask*/ nullptr, /*Flags*/ *Def);
+ Clone->insertAfter(Def);
+ Def->replaceAllUsesWith(Clone);
+ }
+
// Replace wide pointer inductions which have only their scalars used by
// PtrAdd(IndStart, ScalarIVSteps (0, Step)).
if (auto *PtrIV = dyn_cast<VPWidenPointerInductionRecipe>(&Phi)) {
@@ -1522,11 +1547,8 @@ static void narrowToSingleScalarRecipes(VPlan &Plan) {
continue;
}
- // Skip recipes that aren't single scalars and don't just have their first
- // lane used.
- if (!vputils::isSingleScalar(RepOrWidenR) &&
- (!vputils::onlyFirstLaneUsed(RepOrWidenR) ||
- RepOrWidenR->getNumUsers() == 0))
+ // Skip recipes that aren't single scalars.
+ if (!vputils::isSingleScalar(RepOrWidenR))
continue;
// Skip recipes for which conversion to single-scalar does introduce
|
|
@llvm/pr-subscribers-vectorizers Author: Ramkumar Ramachandra (artagnon) ChangesThis reverts commit 7b3ec51, as a crash was reported: https://llvm.godbolt.org/z/dK6ff5zvr -- this will give us time to investigate a re-land. Full diff: https://github.com/llvm/llvm-project/pull/170720.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index b3ba38f6c630e..018c2d21bf46f 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -757,6 +757,31 @@ static void legalizeAndOptimizeInductions(VPlan &Plan) {
if (!PhiR)
continue;
+ // Try to narrow wide and replicating recipes to uniform recipes, based on
+ // VPlan analysis.
+ // TODO: Apply to all recipes in the future, to replace legacy uniformity
+ // analysis.
+ auto Users = collectUsersRecursively(PhiR);
+ for (VPUser *U : reverse(Users)) {
+ auto *Def = dyn_cast<VPRecipeWithIRFlags>(U);
+ auto *RepR = dyn_cast<VPReplicateRecipe>(U);
+ // Skip recipes that shouldn't be narrowed.
+ if (!Def || !isa<VPReplicateRecipe, VPWidenRecipe>(Def) ||
+ Def->getNumUsers() == 0 || !Def->getUnderlyingValue() ||
+ (RepR && (RepR->isSingleScalar() || RepR->isPredicated())))
+ continue;
+
+ // Skip recipes that may have other lanes than their first used.
+ if (!vputils::isSingleScalar(Def) && !vputils::onlyFirstLaneUsed(Def))
+ continue;
+
+ auto *Clone = new VPReplicateRecipe(Def->getUnderlyingInstr(),
+ Def->operands(), /*IsUniform*/ true,
+ /*Mask*/ nullptr, /*Flags*/ *Def);
+ Clone->insertAfter(Def);
+ Def->replaceAllUsesWith(Clone);
+ }
+
// Replace wide pointer inductions which have only their scalars used by
// PtrAdd(IndStart, ScalarIVSteps (0, Step)).
if (auto *PtrIV = dyn_cast<VPWidenPointerInductionRecipe>(&Phi)) {
@@ -1522,11 +1547,8 @@ static void narrowToSingleScalarRecipes(VPlan &Plan) {
continue;
}
- // Skip recipes that aren't single scalars and don't just have their first
- // lane used.
- if (!vputils::isSingleScalar(RepOrWidenR) &&
- (!vputils::onlyFirstLaneUsed(RepOrWidenR) ||
- RepOrWidenR->getNumUsers() == 0))
+ // Skip recipes that aren't single scalars.
+ if (!vputils::isSingleScalar(RepOrWidenR))
continue;
// Skip recipes for which conversion to single-scalar does introduce
|
fhahn
left a comment
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.
LGTM, thanks
This reverts commit 7b3ec51, as a crash was reported: https://llvm.godbolt.org/z/dK6ff5zvr -- this will give us time to investigate a re-land.
This reverts commit 7b3ec51, as a crash was reported: https://llvm.godbolt.org/z/dK6ff5zvr -- this will give us time to investigate a re-land.