diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp --- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp +++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp @@ -8117,7 +8117,7 @@ if (OrigLoop->isLoopExiting(Src)) return EdgeMaskCache[Edge] = SrcMask; - VPValue *EdgeMask = Plan.getOrAddVPValue(BI->getCondition()); + VPValue *EdgeMask = Plan.getOrAddExternalVPValue(BI->getCondition()); assert(EdgeMask && "No Edge Mask found for condition"); if (BI->getSuccessor(0) != Dst) @@ -8128,7 +8128,7 @@ // 'select i1 SrcMask, i1 EdgeMask, i1 false'. // The select version does not introduce new UB if SrcMask is false and // EdgeMask is poison. Using 'and' here introduces undefined behavior. - VPValue *False = Plan.getOrAddVPValue( + VPValue *False = Plan.getOrAddExternalVPValue( ConstantInt::getFalse(BI->getCondition()->getType())); EdgeMask = Builder.createSelect(SrcMask, EdgeMask, False, BI->getDebugLoc()); @@ -8324,7 +8324,7 @@ auto *Phi = cast(I->getOperand(0)); const InductionDescriptor &II = *Legal->getIntOrFpInductionDescriptor(Phi); - VPValue *Start = Plan.getOrAddVPValue(II.getStartValue()); + VPValue *Start = Plan.getOrAddExternalVPValue(II.getStartValue()); return createWidenInductionRecipes(Phi, I, Start, II, CM, Plan, *PSE.getSE(), *OrigLoop, Range); } @@ -8457,7 +8457,7 @@ if (Legal->isMaskRequired(CI)) Mask = createBlockInMask(CI->getParent(), *Plan); else - Mask = Plan->getOrAddVPValue(ConstantInt::getTrue( + Mask = Plan->getOrAddExternalVPValue(ConstantInt::getTrue( IntegerType::getInt1Ty(Variant->getFunctionType()->getContext()))); VFShape Shape = VFShape::get(*CI, VariantVF, /*HasGlobalPred=*/true); @@ -8509,8 +8509,8 @@ if (CM.isPredicatedInst(I)) { SmallVector Ops(Operands.begin(), Operands.end()); VPValue *Mask = createBlockInMask(I->getParent(), *Plan); - VPValue *One = - Plan->getOrAddExternalDef(ConstantInt::get(I->getType(), 1u, false)); + VPValue *One = Plan->getOrAddExternalVPValue( + ConstantInt::get(I->getType(), 1u, false)); auto *SafeRHS = new VPInstruction(Instruction::Select, {Mask, Ops[1], One}, I->getDebugLoc()); @@ -8730,7 +8730,7 @@ static void addCanonicalIVRecipes(VPlan &Plan, Type *IdxTy, DebugLoc DL, TailFoldingStyle Style) { Value *StartIdx = ConstantInt::get(IdxTy, 0); - auto *StartV = Plan.getOrAddVPValue(StartIdx); + auto *StartV = Plan.getOrAddExternalVPValue(StartIdx); // Add a VPCanonicalIVPHIRecipe starting at 0 to the header. auto *CanonicalIVPHI = new VPCanonicalIVPHIRecipe(StartV, DL); @@ -8846,7 +8846,7 @@ for (PHINode &ExitPhi : ExitBB->phis()) { Value *IncomingValue = ExitPhi.getIncomingValueForBlock(ExitingBB); - VPValue *V = Plan.getOrAddVPValue(IncomingValue, true); + VPValue *V = Plan.getOrAddExternalVPValue(IncomingValue, true); Plan.addLiveOut(&ExitPhi, V); } } @@ -8957,7 +8957,7 @@ SmallVector Operands; auto *Phi = dyn_cast(Instr); if (Phi && Phi->getParent() == OrigLoop->getHeader()) { - Operands.push_back(Plan->getOrAddVPValue( + Operands.push_back(Plan->getOrAddExternalVPValue( Phi->getIncomingValueForBlock(OrigLoop->getLoopPreheader()))); } else { auto OpRange = Plan->mapToVPValues(Instr->operands()); @@ -10446,7 +10446,8 @@ IndPhi, *ID, {EPI.MainLoopIterationCountCheck}); } assert(ResumeV && "Must have a resume value"); - VPValue *StartVal = BestEpiPlan.getOrAddExternalDef(ResumeV); + VPValue *StartVal = BestEpiPlan.getOrAddExternalVPValue( + ResumeV, /*OverrideAllowed=*/true); cast(&R)->setStartValue(StartVal); } diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h --- a/llvm/lib/Transforms/Vectorize/VPlan.h +++ b/llvm/lib/Transforms/Vectorize/VPlan.h @@ -2227,10 +2227,6 @@ /// Holds the name of the VPlan, for printing. std::string Name; - /// Holds all the external definitions created for this VPlan. External - /// definitions must be immutable and hold a pointer to their underlying IR. - DenseMap VPExternalDefs; - /// Represents the trip count of the original loop, for folding /// the tail. VPValue *TripCount = nullptr; @@ -2246,9 +2242,9 @@ /// VPlan. Value2VPValueTy Value2VPValue; - /// Contains all VPValues that been allocated by addVPValue directly and need - /// to be free when the plan's destructor is called. - SmallVector VPValuesToFree; + /// Contains all the external definitions created for this VPlan. External + /// definitions are VPValues that hold a pointer to their underlying IR. + SmallVector VPExternalDefs; /// Indicates whether it is safe use the Value2VPValue mapping or if the /// mapping cannot be used any longer, because it is stale. @@ -2328,24 +2324,6 @@ void setName(const Twine &newName) { Name = newName.str(); } - /// Get the existing or add a new external definition for \p V. - VPValue *getOrAddExternalDef(Value *V) { - auto I = VPExternalDefs.insert({V, nullptr}); - if (I.second) - I.first->second = new VPValue(V); - return I.first->second; - } - - void addVPValue(Value *V) { - assert(Value2VPValueEnabled && - "IR value to VPValue mapping may be out of date!"); - assert(V && "Trying to add a null Value to VPlan"); - assert(!Value2VPValue.count(V) && "Value already exists in VPlan"); - VPValue *VPV = new VPValue(V); - Value2VPValue[V] = VPV; - VPValuesToFree.push_back(VPV); - } - void addVPValue(Value *V, VPValue *VPV) { assert(Value2VPValueEnabled && "Value2VPValue mapping may be out of date!"); assert(V && "Trying to add a null Value to VPlan"); @@ -2363,16 +2341,27 @@ return Value2VPValue[V]; } - /// Gets the VPValue or adds a new one (if none exists yet) for \p V. \p - /// OverrideAllowed can be used to disable checking whether it is safe to - /// query VPValues using IR Values. - VPValue *getOrAddVPValue(Value *V, bool OverrideAllowed = false) { - assert((OverrideAllowed || isa(V) || Value2VPValueEnabled) && - "Value2VPValue mapping may be out of date!"); + /// Adds a VPValue wrapping \p V as external definition. + void addExternalVPValue(Value *V) { + assert(V && "Trying to add a null Value to VPlan"); + assert(!Value2VPValue.count(V) && "Value already exists in VPlan"); + VPValue *VPV = new VPValue(V); + Value2VPValue[V] = VPV; + VPExternalDefs.push_back(VPV); + } + + /// Gets the VPValue for \p V or adds a new external definition (if none + /// exists yet) for \p V. \p OverrideAllowed can be used to disable checking + /// whether it is safe to query VPValues using IR Values. + VPValue *getOrAddExternalVPValue(Value *V, bool OverrideAllowed = false) { assert(V && "Trying to get or add the VPValue of a null Value"); if (!Value2VPValue.count(V)) - addVPValue(V); - return getVPValue(V); + addExternalVPValue(V); + VPValue *VPV = Value2VPValue[V]; + assert((OverrideAllowed || Value2VPValueEnabled || + !VPV->getDefiningRecipe()) && + "IR value to VPValue mapping may be out of date!"); + return VPV; } void removeVPValueFor(Value *V) { @@ -2397,7 +2386,7 @@ iterator_range>> mapToVPValues(User::op_range Operands) { std::function Fn = [this](Value *Op) { - return getOrAddVPValue(Op); + return getOrAddExternalVPValue(Op); }; return map_range(Operands, Fn); } diff --git a/llvm/lib/Transforms/Vectorize/VPlan.cpp b/llvm/lib/Transforms/Vectorize/VPlan.cpp --- a/llvm/lib/Transforms/Vectorize/VPlan.cpp +++ b/llvm/lib/Transforms/Vectorize/VPlan.cpp @@ -591,14 +591,12 @@ VPBlockBase::deleteCFG(Entry); } - for (VPValue *VPV : VPValuesToFree) + for (VPValue *VPV : VPExternalDefs) delete VPV; if (TripCount) delete TripCount; if (BackedgeTakenCount) delete BackedgeTakenCount; - for (auto &P : VPExternalDefs) - delete P.second; } VPActiveLaneMaskPHIRecipe *VPlan::getActiveLaneMaskPhi() { @@ -641,7 +639,7 @@ // needs to be changed from zero to the value after the main vector loop. // FIXME: Improve modeling for canonical IV start values in the epilogue loop. if (CanonicalIVStartValue) { - VPValue *VPV = getOrAddExternalDef(CanonicalIVStartValue); + VPValue *VPV = getOrAddExternalVPValue(CanonicalIVStartValue); auto *IV = getCanonicalIV(); assert(all_of(IV->users(), [](const VPUser *U) { @@ -1131,9 +1129,9 @@ VPValue *vputils::getOrCreateVPValueForSCEVExpr(VPlan &Plan, const SCEV *Expr, ScalarEvolution &SE) { if (auto *E = dyn_cast(Expr)) - return Plan.getOrAddExternalDef(E->getValue()); + return Plan.getOrAddExternalVPValue(E->getValue()); if (auto *E = dyn_cast(Expr)) - return Plan.getOrAddExternalDef(E->getValue()); + return Plan.getOrAddExternalVPValue(E->getValue()); VPBasicBlock *Preheader = Plan.getEntry()->getEntryBasicBlock(); VPExpandSCEVRecipe *Step = new VPExpandSCEVRecipe(Expr, SE); diff --git a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp --- a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp +++ b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp @@ -196,7 +196,7 @@ // A and B: Create VPValue and add it to the pool of external definitions and // to the Value->VPValue map. - VPValue *NewVPVal = Plan.getOrAddExternalDef(IRVal); + VPValue *NewVPVal = Plan.getOrAddExternalVPValue(IRVal); IRDef2VPValue[IRVal] = NewVPVal; return NewVPVal; } @@ -272,7 +272,7 @@ for (auto &I : *ThePreheaderBB) { if (I.getType()->isVoidTy()) continue; - IRDef2VPValue[&I] = Plan.getOrAddExternalDef(&I); + IRDef2VPValue[&I] = Plan.getOrAddExternalVPValue(&I); } // Create empty VPBB for Loop H so that we can link PH->H. VPBlockBase *HeaderVPBB = getOrCreateVPBB(TheLoop->getHeader()); diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp --- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp +++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp @@ -52,7 +52,7 @@ if (auto *VPPhi = dyn_cast(&Ingredient)) { auto *Phi = cast(VPPhi->getUnderlyingValue()); if (const auto *II = GetIntOrFpInductionDescriptor(Phi)) { - VPValue *Start = Plan->getOrAddVPValue(II->getStartValue()); + VPValue *Start = Plan->getOrAddExternalVPValue(II->getStartValue()); VPValue *Step = vputils::getOrCreateVPValueForSCEVExpr(*Plan, II->getStep(), SE); NewRecipe = @@ -68,13 +68,15 @@ // Create VPWidenMemoryInstructionRecipe for loads and stores. if (LoadInst *Load = dyn_cast(Inst)) { NewRecipe = new VPWidenMemoryInstructionRecipe( - *Load, Plan->getOrAddVPValue(getLoadStorePointerOperand(Inst)), + *Load, + Plan->getOrAddExternalVPValue(getLoadStorePointerOperand(Inst)), nullptr /*Mask*/, false /*Consecutive*/, false /*Reverse*/); } else if (StoreInst *Store = dyn_cast(Inst)) { NewRecipe = new VPWidenMemoryInstructionRecipe( - *Store, Plan->getOrAddVPValue(getLoadStorePointerOperand(Inst)), - Plan->getOrAddVPValue(Store->getValueOperand()), nullptr /*Mask*/, - false /*Consecutive*/, false /*Reverse*/); + *Store, + Plan->getOrAddExternalVPValue(getLoadStorePointerOperand(Inst)), + Plan->getOrAddExternalVPValue(Store->getValueOperand()), + nullptr /*Mask*/, false /*Consecutive*/, false /*Reverse*/); } else if (GetElementPtrInst *GEP = dyn_cast(Inst)) { NewRecipe = new VPWidenGEPRecipe(GEP, Plan->mapToVPValues(GEP->operands())); @@ -599,9 +601,9 @@ return; LLVMContext &Ctx = SE.getContext(); - auto *BOC = - new VPInstruction(VPInstruction::BranchOnCond, - {Plan.getOrAddExternalDef(ConstantInt::getTrue(Ctx))}); + auto *BOC = new VPInstruction( + VPInstruction::BranchOnCond, + {Plan.getOrAddExternalVPValue(ConstantInt::getTrue(Ctx))}); Term->eraseFromParent(); ExitingVPBB->appendRecipe(BOC); Plan.setVF(BestVF); diff --git a/llvm/unittests/Transforms/Vectorize/VPlanHCFGTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanHCFGTest.cpp --- a/llvm/unittests/Transforms/Vectorize/VPlanHCFGTest.cpp +++ b/llvm/unittests/Transforms/Vectorize/VPlanHCFGTest.cpp @@ -96,7 +96,7 @@ #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP) // Add an external value to check we do not print the list of external values, // as this is not required with the new printing. - Plan->addVPValue(&*F->arg_begin()); + Plan->addExternalVPValue(&*F->arg_begin()); std::string FullDump; raw_string_ostream OS(FullDump); Plan->printDOT(OS); diff --git a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp --- a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp +++ b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp @@ -1203,8 +1203,8 @@ BinaryOperator::CreateAdd(UndefValue::get(Int32), UndefValue::get(Int32)); AI->setName("a"); SmallVector Args; - VPValue *ExtVPV1 = Plan.getOrAddExternalDef(ConstantInt::get(Int32, 1)); - VPValue *ExtVPV2 = Plan.getOrAddExternalDef(ConstantInt::get(Int32, 2)); + VPValue *ExtVPV1 = Plan.getOrAddExternalVPValue(ConstantInt::get(Int32, 1)); + VPValue *ExtVPV2 = Plan.getOrAddExternalVPValue(ConstantInt::get(Int32, 2)); Args.push_back(ExtVPV1); Args.push_back(ExtVPV2); VPWidenRecipe *WidenR =