[VPlan] Consolidate logic for narrowToSingleScalars (NFC)

[artagnon]

The logic for narrowing to single scalar recipes is in two different places: narrowToSingleScalarRecipes and legalizeAndOptimizeInductions. Consolidate them.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-vectorizers

Author: Ramkumar Ramachandra (artagnon)

Changes

The logic for narrowing to single scalar recipes is in two different places: narrowToSingleScalarRecipes and legalizeAndOptimizeInductions. Consolidate them.

---

Full diff: https://github.com/llvm/llvm-project/pull/167360.diff

1 Files Affected:

• (modified) llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp (+3-25)

diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index f5bef08fafcdc..77fd6203016ae 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -670,30 +670,6 @@ 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<VPSingleDefRecipe>(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);
-      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)) {
@@ -1418,7 +1394,9 @@ static void narrowToSingleScalarRecipes(VPlan &Plan) {
       // Skip recipes that aren't single scalars or don't have only their
       // scalar results used. In the latter case, we would introduce extra
       // broadcasts.
-      if (!vputils::isSingleScalar(RepOrWidenR) ||
+      if ((!vputils::isSingleScalar(RepOrWidenR) &&
+           !vputils::onlyFirstLaneUsed(RepOrWidenR)) ||
+          RepOrWidenR->getNumUsers() == 0 ||
           !all_of(RepOrWidenR->users(), [RepOrWidenR](const VPUser *U) {
             if (auto *Store = dyn_cast<VPWidenStoreRecipe>(U)) {
               // VPWidenStore doesn't have users, and stores are always

[lukel97]

Is adding this check to skip dead recipes required for this PR? I see we did it in legalizeAndOptimizeInductions beforehand but I'm not sure why its needed

[artagnon]

Yes, it is needed to exclude replicate stores?

[lukel97]

I'm confused as to why narrowToSingleScalarRecipes didn't need it beforehand.

If I'm reading this correctly, if a replicate store is a single scalar then we will exclude it on line 1373 above. If it's not a single scalar then we will exclude it anyway with the !vputils::isSingleScalar(RepOrWidenR) && !vputils::onlyFirstLaneUsed(RepOrWidenR) check above.

[artagnon]

(Sorry, still waking up, so kindly excuse mistakes)

I think the problem is that vputils::onlyFirstLaneUsed returns true for replicate stores, as they have no users, and hence all users use the first lane one only?

[lukel97]

Ah right. In that case then why didn't we need to check !vputils::onlyFirstLaneUsed beforehand?

[artagnon]

I think that it was less powerful before (just an optimization issue, not a correctness one), and the code we removed now was doing part of the job?

[github-actions]

🐧 Linux x64 Test Results

• 186424 tests passed
• 4863 tests skipped