[VPlan] Move Load/Store checks out of tryToWiden (NFC).

[fhahn]

Summary

Handling LoadInst and StoreInst in tryToWiden seems a bit
counter-intuitive, as there is only an assertion for them and in no
case VPWidenRefipes are created for them.

I think it makes sense to move the assertion to handleReplication, where
the non-widened loads and store are handled.

[Ayal]

Thanks for this cleanup!

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

6993

Another cleanup: remove PHI from this list and replace it with an assert(!isa<PHINode>(I)) at the outset, as PHIs are all handled by other, earlier recipes. This will further simplify willWiden() below, which can also absorb IsPredicated above: for non IsVectorizableOpcodes, handleReplication() will clamp the range according to IsPredicated anyhow.

(That will in turn lead to identical willWiden() across tryToWiden, tryToWidenSelect, and the inlined tryToWidenGEP ("Scalarize"); which suggests folding/further cleanup.)

7059

tryToWidenMemoryInstruction() is expected to have clamped the Range already; if not, Range should either be asserted to be clamped (w/o clamping) or it should be clamped (redundantly) but not under assert. Note that checking VF==1, Load or Store should be done alongside IsPredicated, outside getDecisionAndClampRange(), as they're independent of Range.

This assert, moved here from tryToWiden(), is practically the complementary of tryToWidenMemoryInstruction()'s willWiden(). Due to its complexity and redundancy, it should probably simply be removed.

[fhahn]

Also remove PHI from vectorizable opcodes, remove assertion in handleReplication.

[fhahn]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

7059

Yeah, initially I was hoping there would be a general way to assert that we only call handleReplication with instructions we expect to scalarize, but it seems like there is no general way to write such an assertion. The assertion I added is not that helpful in that regard, I've removed it as suggested.

[Ayal]

Looks good to me, thanks!

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

7021

clang-format?

[fhahn]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

7021

Fixed in committed version.

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
 //
 //===----------------------------------------------------------------------===//
 //
auto IsVectorizableOpcode = [](unsigned Opcode) {
     case Instruction::FPExt:
     case Instruction::FPToSI:
     case Instruction::FPToUI:
     case Instruction::FPTrunc:
     case Instruction::FRem:
     case Instruction::FSub:
     case Instruction::ICmp:
     case Instruction::IntToPtr:
-    case Instruction::Load:
     case Instruction::LShr:
     case Instruction::Mul:
     case Instruction::Or:
     case Instruction::PHI:
     case Instruction::PtrToInt:
     case Instruction::SDiv:
     case Instruction::Select:
     case Instruction::SExt:
     case Instruction::Shl:
     case Instruction::SIToFP:
     case Instruction::SRem:
-    case Instruction::Store:
     case Instruction::Sub:
     case Instruction::Trunc:
     case Instruction::UDiv:
     case Instruction::UIToFP:
     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;
-    if (isa<LoadInst>(I) || isa<StoreInst>(I)) {
-      assert(CM.getWideningDecision(I, VF) ==
-                 LoopVectorizationCostModel::CM_Scalarize &&
-             "Memory widening decisions should have been taken care by now");
-      return false;
-    }
     return true;
   };
 
   if (!LoopVectorizationPlanner::getDecisionAndClampRange(willWiden, 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);
 }
 
 VPBasicBlock *VPRecipeBuilder::handleReplication(
     Instruction *I, VFRange &Range, VPBasicBlock *VPBB,
     DenseMap<Instruction *, VPReplicateRecipe *> &PredInst2Recipe,
     VPlanPtr &Plan) {
   bool IsUniform = LoopVectorizationPlanner::getDecisionAndClampRange(
       [&](unsigned VF) { return CM.isUniformAfterVectorization(I, VF); },
       Range);
 
   bool IsPredicated = LoopVectorizationPlanner::getDecisionAndClampRange(
       [&](unsigned VF) { return CM.isScalarWithPredication(I, VF); }, Range);
 
+  // Make sure only memory instructions explicitly marked or profitable with
+  // scalarization are handled with replication. Otherwise they should have been
+  // widened earlier.
+  assert(
+      IsPredicated ||
+      LoopVectorizationPlanner::getDecisionAndClampRange(
+          [this, I](unsigned VF) {
+            if (VF == 1 || (!isa<LoadInst>(I) && !isa<StoreInst>(I)))
+              return true;
+            return CM.isProfitableToScalarize(I, VF) ||
+                   CM.isScalarAfterVectorization(I, VF) ||
+                   CM.getWideningDecision(I, VF) ==
+                       LoopVectorizationCostModel::CM_Scalarize;
+          },
+          Range) &&
+          "Memory widening decisions should have been taken care of by now.");
+
   auto *Recipe = new VPReplicateRecipe(I, IsUniform, IsPredicated);
   setRecipe(I, Recipe);
 
   // Find if I uses a predicated instruction. If so, it will use its scalar
   // value. Avoid hoisting the insert-element which packs the scalar value into
   // a vector value, as that happens iff all users use the vector value.
   for (auto &Op : I->operands())
     if (auto *PredInst = dyn_cast<Instruction>(Op))