[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

6987–6988

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.)

7048

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

7048

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

7012

clang-format?

[fhahn]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

7012

Fixed in committed version.

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

 
 VPWidenRecipe *VPRecipeBuilder::tryToWiden(Instruction *I, VFRange &Range) {
   bool IsPredicated = LoopVectorizationPlanner::getDecisionAndClampRange(
       [&](unsigned VF) { return CM.isScalarWithPredication(I, VF); }, Range);
 
   if (IsPredicated)
     return nullptr;
 
+  assert(!isa<PHINode>(I) && "PHIs should have been handled earlier");
   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:
     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;
+    return !CM.isScalarAfterVectorization(I, VF) &&
+           !CM.isProfitableToScalarize(I, VF);
   };
 
   if (!LoopVectorizationPlanner::getDecisionAndClampRange(willWiden, Range))
     return nullptr;
 
   // Success: widen this instruction.
   return new VPWidenRecipe(*I);
 }
for (auto &Op : I->operands())
     if (auto *PredInst = dyn_cast<Instruction>(Op))
       if (PredInst2Recipe.find(PredInst) != PredInst2Recipe.end())
         PredInst2Recipe[PredInst]->setAlsoPack(false);
 
   // Finalize the recipe for Instr, first if it is not predicated.
   if (!IsPredicated) {
     LLVM_DEBUG(dbgs() << "LV: Scalarizing:" << *I << "\n");
     VPBB->appendRecipe(Recipe);
     return VPBB;
   }
   LLVM_DEBUG(dbgs() << "LV: Scalarizing and predicating:" << *I << "\n");
   assert(VPBB->getSuccessors().empty() &&
          "VPBB has successors when handling predicated replication.");
   // Record predicated instructions for above packing optimizations.
   PredInst2Recipe[I] = Recipe;
   VPBlockBase *Region = createReplicateRegion(I, Recipe, Plan);
   VPBlockUtils::insertBlockAfter(Region, VPBB);