[VPlan] Move widening check for non-memory/non-calls to function (NFC).

[fhahn]

Summary

After introducing VPWidenSelectRecipe, the duplicated logic can be
shared.

[Ayal]

Yes, this follows D77972 naturally :-)

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

6940

Can add !isa<PHINode>(I) to this assertion list after sinking tryToWidenSelect() as suggested below.

7122

How about doing here (tryToCreateRecipe should have a better name btw):

  if (!shouldWiden(I, Range))
    return false;

  if ((Recipe = tryToWidenSelect(Instr)) ||
      (isa<GetElementPtrInst>(Instr) &&
       (Recipe = new VPWidenGEPRecipe(GEP, OrigLoop)))) ||
     (Recipe = tryToWiden(Instr, *Plan))) {
    setRecipe(Instr, Recipe);
    VPBB->appendRecipe(Recipe);
    return true;
  }

  return false;
}

[fhahn]

Consolidate widening checks for GEP, Select and general widening as suggested in tryToCreateRecipe.

[Ayal]

Thanks for cleaning this up! Final couple of nits.

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

6955

Retain the !isa<Call, Load, Store> alongside !isa<PHI>?
To avoid using this method for such opcodes, as their widening decision is more complex.

6957

More natural to reverse the conditions and rename willWiden to willScalarize, returning !getDecisionAndClampRange(willScalarize, Range)?

6973

Branches should also find their way out of IsVectorizableOpcode and into an assert; no recipes are created for them.

[fhahn]

Change willWiden to willScalarize.

(This patch depends on D77973)

[fhahn]

Avoid calling shouldWiden for calls, loads and stores by exiting earlier. They should be widened by the specialized recipes or scalarized.

[fhahn]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

6955

I've brought it back, but with the new code structure, it also required adding an early exit for calls, loads & stores, but I think that makes sense. What do you think?

6973

Will submit as a follow-up commit once this one lands.

[Ayal]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

6955

Ahh, right. Perhaps better to early-exit here:

// Calls and memory instructions that were not widened earlier should be scalarized.
if (isa<CallInst>(Instr) || isa<LoadInst>(Instr) || isa<StoreInst>(Instr))
  return false;
assert(!isa<PHINode>(I) && ...);

6973

Sure.

[fhahn]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

6955

Is there an advantage to have the early exit here? To me it seems to fit slightly better at the call site of shouldWiden, because there it is clearer what is happening (i.e. the code to handle calls, stores, loads is just above.)

[Ayal]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

6955

Your original version is fine, as approved. Advantage of last suggestion was merely to avoid an obvious assert immediately following an early-exiting.

Thoughts of further cleanup:

• have tryToCreate[Widen]Recipe return a VPRecipeBase* instead of bool,
• have its caller take care of { setRecipe(Instr, Recipe); VPBB->appendRecipe(Recipe); }; either using another method of VPRecipeBuilder or exposing its setRecipe() to LVP.
• split tryToOptimizeInduction in two: first part that handles PHI and second that handles Trunc
• refactor out various isa's from tryTo's and fold the early-exits closer:

VPRecipeBase *VPRecipeBuilder::tryToCreateWidenRecipe(Instruction *Instr, VFRange &Range, VPlanPtr &Plan) {
  if (auto *CI = dyn_cast<CallInst>(Instr))
    return tryToWidenCall(CI, Range, *Plan);
  if (isa<LoadInst>(Instr) || isa<StoreInst>(Instr))
    return tryToWidenMemory(Instr, Range, Plan);
  if (auto *PHI = dyn_cast<PHINode>(Instr)) {
    if (auto *Recipe = tryToOptimizeInduction(PHI, Range))
      return Recipe;
    if (auto *Recipe = tryToBlend(PHI, Plan))
      return Recipe;
    return new VPWidenPHIRecipe(PHI);
  }
  if (auto *Trunc = dyn_cast<TruncInst>(Instr))
    if (auto *Recipe = tryToOptimizeIVTruncate(Trunc, Range))
      return Recipe;
  
  if (!shouldWiden(Instr, Range))
    return nullptr;

  if (auto *GEP = dyn_cast<GetElementPtrInst>(Instr))
    return new VPWidenGEPRecipe(GEP, OrigLoop);
  if (auto *Select = dyn_cast<SelectInst<(Instr))
    return tryToWidenSelect(Instr);
  return tryToWiden(Instr, *Plan);
}

[fhahn]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

6955

SGTM, I just wanted to keep this relatively self-contained. It would be great if you could also take another look at D77972, which is required for this patch to land :)

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

 //===- LoopVectorize.cpp - A Loop Vectorizer ------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
VPRecipeBuilder::tryToWidenCall(Instruction *I, VFRange &Range, VPlan &Plan) {
   // Success: widen this call.
   auto VPValues = map_range(CI->arg_operands(), [&Plan](Value *Op) {
     return Plan.getOrAddVPValue(Op);
   });
 
   return new VPWidenCallRecipe(*CI, VPValues);
 }
 
+bool VPRecipeBuilder::shouldWiden(Instruction *I, VFRange &Range) const {
+  assert(!isa<CallInst>(I) && !isa<StoreInst>(I) && !isa<LoadInst>(I) &&
+         "Called with unsupported instruction");
+  // Instruction should be widened, unless it is scalar after vectorization,
+  // scalarization is profitable or it is predicated.
+  auto willWiden = [this, I](unsigned VF) -> bool {
+    return !CM.isScalarAfterVectorization(I, VF) &&
+           !CM.isProfitableToScalarize(I, VF) &&
+           !CM.isScalarWithPredication(I, VF);
+  };
+  return LoopVectorizationPlanner::getDecisionAndClampRange(willWiden, Range);
+}
+
 VPWidenSelectRecipe *VPRecipeBuilder::tryToWidenSelect(Instruction *I,
                                                        VFRange &Range) {
   auto *SI = dyn_cast<SelectInst>(I);
-  if (!SI)
-    return nullptr;
-
-  // SI should be widened, unless it is scalar after vectorization,
-  // scalarization is profitable or it is predicated.
-  auto willWiden = [this, SI](unsigned VF) -> bool {
-    return !CM.isScalarAfterVectorization(SI, VF) &&
-           !CM.isProfitableToScalarize(SI, VF) &&
-           !CM.isScalarWithPredication(SI, VF);
-  };
-  if (!LoopVectorizationPlanner::getDecisionAndClampRange(willWiden, Range))
+  if (!SI || !shouldWiden(I, Range))
     return nullptr;
 
   auto *SE = PSE.getSE();
   bool InvariantCond =
       SE->isLoopInvariant(PSE.getSCEV(SI->getOperand(0)), OrigLoop);
   // Success: widen this instruction.
   return new VPWidenSelectRecipe(*SI, InvariantCond);
 }
 
 VPWidenRecipe *VPRecipeBuilder::tryToWiden(Instruction *I, VFRange &Range,
                                            VPlan &Plan) {
-  bool IsPredicated = LoopVectorizationPlanner::getDecisionAndClampRange(
-      [&](unsigned VF) { return CM.isScalarWithPredication(I, VF); }, Range);
-
-  if (IsPredicated)
-    return nullptr;
-
   auto IsVectorizableOpcode = [](unsigned Opcode) {
     switch (Opcode) {
     case Instruction::Add:
     case Instruction::And:
     case Instruction::AShr:
     case Instruction::BitCast:
     case Instruction::Br:
     case Instruction::FAdd:
     case Instruction::FCmp:
     case Instruction::FDiv:
     case Instruction::FMul:
     case Instruction::FNeg:
     case Instruction::FPExt:
     case Instruction::FPToSI:
     case Instruction::FPToUI:
auto IsVectorizableOpcode = [](unsigned Opcode) {
     case Instruction::URem:
     case Instruction::Xor:
     case Instruction::ZExt:
       return true;
     }
     return false;
   };
 
-  if (!IsVectorizableOpcode(I->getOpcode()))
-    return nullptr;
-
-  auto willWiden = [&](unsigned VF) -> bool {
-    if (!isa<PHINode>(I) && (CM.isScalarAfterVectorization(I, VF) ||
-                             CM.isProfitableToScalarize(I, VF)))
-      return false;
-    return true;
-  };
-
-  if (!LoopVectorizationPlanner::getDecisionAndClampRange(willWiden, Range))
+  if (!IsVectorizableOpcode(I->getOpcode()) || !shouldWiden(I, Range))
     return nullptr;
 
   // Success: widen this instruction.
   // Create VPValue operands.
   auto VPValues = map_range(
       I->operands(), [&Plan](Value *Op) { return Plan.getOrAddVPValue(Op); });
   return new VPWidenRecipe(*I, VPValues);
 }
if ((Recipe = tryToWidenCall(Instr, Range, *Plan)) ||
       (Recipe = tryToOptimizeInduction(Instr, Range)) ||
       (Recipe = tryToBlend(Instr, Plan)) ||
       (isa<PHINode>(Instr) &&
        (Recipe = new VPWidenPHIRecipe(cast<PHINode>(Instr))))) {
     setRecipe(Instr, Recipe);
     VPBB->appendRecipe(Recipe);
     return true;
   }
 
   // Handle GEP widening.
   if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(Instr)) {
     auto Scalarize = [&](unsigned VF) {
       return CM.isScalarWithPredication(Instr, VF) ||
              CM.isScalarAfterVectorization(Instr, VF) ||
              CM.isProfitableToScalarize(Instr, VF);
     };
     if (LoopVectorizationPlanner::getDecisionAndClampRange(Scalarize, Range))

llvm/lib/Transforms/Vectorize/VPRecipeBuilder.h

 //===- VPRecipeBuilder.h - Helper class to build recipes --------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
class VPRecipeBuilder {
   void setRecipe(Instruction *I, VPRecipeBase *R) {
     if (!Ingredient2Recipe.count(I))
       return;
     assert(Ingredient2Recipe[I] == nullptr &&
            "Recipe already set for ingredient");
     Ingredient2Recipe[I] = R;
   }
 
+  /// Check if \p I can be widened at the start of \p Range and possibly
+  /// decrease the range such that the returned value holds for the entire \p
+  /// Range. The function should not be called for memory instructions or calls.
+  bool shouldWiden(Instruction *I, VFRange &Range) const;
+
 public:
   /// A helper function that computes the predicate of the block BB, assuming
   /// that the header block of the loop is set to True. It returns the *entry*
   /// mask for the block BB.
   VPValue *createBlockInMask(BasicBlock *BB, VPlanPtr &Plan);
 
   /// A helper function that computes the predicate of the edge between SRC
   /// and DST.