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 @@ -8146,7 +8146,7 @@ if (OrigLoop->isLoopExiting(Src)) return EdgeMaskCache[Edge] = SrcMask; - VPValue *EdgeMask = Plan.getOrAddVPValue(BI->getCondition()); + VPValue *EdgeMask = Plan.getVPValueOrAddLiveIn(BI->getCondition()); assert(EdgeMask && "No Edge Mask found for condition"); if (BI->getSuccessor(0) != Dst) @@ -8157,7 +8157,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.getVPValueOrAddLiveIn( ConstantInt::getFalse(BI->getCondition()->getType())); EdgeMask = Builder.createSelect(SrcMask, EdgeMask, False, BI->getDebugLoc()); @@ -8353,7 +8353,7 @@ auto *Phi = cast(I->getOperand(0)); const InductionDescriptor &II = *Legal->getIntOrFpInductionDescriptor(Phi); - VPValue *Start = Plan.getOrAddVPValue(II.getStartValue()); + VPValue *Start = Plan.getVPValueOrAddLiveIn(II.getStartValue()); return createWidenInductionRecipes(Phi, I, Start, II, CM, Plan, *PSE.getSE(), *OrigLoop, Range); } @@ -8486,7 +8486,7 @@ if (Legal->isMaskRequired(CI)) Mask = createBlockInMask(CI->getParent(), *Plan); else - Mask = Plan->getOrAddVPValue(ConstantInt::getTrue( + Mask = Plan->getVPValueOrAddLiveIn(ConstantInt::getTrue( IntegerType::getInt1Ty(Variant->getFunctionType()->getContext()))); VFShape Shape = VFShape::get(*CI, VariantVF, /*HasGlobalPred=*/true); @@ -8538,8 +8538,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->getVPValueOrAddLiveIn( + ConstantInt::get(I->getType(), 1u, false)); auto *SafeRHS = new VPInstruction(Instruction::Select, {Mask, Ops[1], One}, I->getDebugLoc()); @@ -8760,7 +8760,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.getVPValueOrAddLiveIn(StartIdx); // Add a VPCanonicalIVPHIRecipe starting at 0 to the header. auto *CanonicalIVPHI = new VPCanonicalIVPHIRecipe(StartV, DL); @@ -8876,7 +8876,7 @@ for (PHINode &ExitPhi : ExitBB->phis()) { Value *IncomingValue = ExitPhi.getIncomingValueForBlock(ExitingBB); - VPValue *V = Plan.getOrAddVPValue(IncomingValue, true); + VPValue *V = Plan.getVPValueOrAddLiveIn(IncomingValue); Plan.addLiveOut(&ExitPhi, V); } } @@ -8987,7 +8987,7 @@ SmallVector Operands; auto *Phi = dyn_cast(Instr); if (Phi && Phi->getParent() == OrigLoop->getHeader()) { - Operands.push_back(Plan->getOrAddVPValue( + Operands.push_back(Plan->getVPValueOrAddLiveIn( Phi->getIncomingValueForBlock(OrigLoop->getLoopPreheader()))); } else { auto OpRange = Plan->mapToVPValues(Instr->operands()); @@ -10477,7 +10477,7 @@ IndPhi, *ID, {EPI.MainLoopIterationCountCheck}); } assert(ResumeV && "Must have a resume value"); - VPValue *StartVal = BestEpiPlan.getOrAddExternalDef(ResumeV); + VPValue *StartVal = BestEpiPlan.getVPValueOrAddLiveIn(ResumeV); 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 @@ -2233,10 +2233,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; @@ -2252,9 +2248,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 VPLiveInsToFree; /// Indicates whether it is safe use the Value2VPValue mapping or if the /// mapping cannot be used any longer, because it is stale. @@ -2334,50 +2330,35 @@ 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((Value2VPValueEnabled || !VPV->getDefiningRecipe()) && + "Value2VPValue 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"); Value2VPValue[V] = VPV; } /// Returns the VPValue for \p V. \p OverrideAllowed can be used to disable - /// checking whether it is safe to query VPValues using IR Values. + /// /// checking whether it is safe to query VPValues using IR Values. VPValue *getVPValue(Value *V, bool OverrideAllowed = false) { - assert((OverrideAllowed || isa(V) || Value2VPValueEnabled) && - "Value2VPValue mapping may be out of date!"); assert(V && "Trying to get the VPValue of a null Value"); assert(Value2VPValue.count(V) && "Value does not exist in VPlan"); + assert((!Value2VPValue[V]->getDefiningRecipe() || Value2VPValueEnabled || + OverrideAllowed) && + "Value2VPValue mapping may be out of date!"); 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!"); + /// Gets the VPValue for \p V or adds a new live-in (if none exists yet) for + /// \p V. + VPValue *getVPValueOrAddLiveIn(Value *V) { assert(V && "Trying to get or add the VPValue of a null Value"); - if (!Value2VPValue.count(V)) - addVPValue(V); + if (!Value2VPValue.count(V)) { + VPValue *VPV = new VPValue(V); + VPLiveInsToFree.push_back(VPV); + addVPValue(V, VPV); + } + return getVPValue(V); } @@ -2403,7 +2384,7 @@ iterator_range>> mapToVPValues(User::op_range Operands) { std::function Fn = [this](Value *Op) { - return getOrAddVPValue(Op); + return getVPValueOrAddLiveIn(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 : VPLiveInsToFree) 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 = getVPValueOrAddLiveIn(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.getVPValueOrAddLiveIn(E->getValue()); if (auto *E = dyn_cast(Expr)) - return Plan.getOrAddExternalDef(E->getValue()); + return Plan.getVPValueOrAddLiveIn(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.getVPValueOrAddLiveIn(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.getVPValueOrAddLiveIn(&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->getVPValueOrAddLiveIn(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->getVPValueOrAddLiveIn(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->getVPValueOrAddLiveIn(getLoadStorePointerOperand(Inst)), + Plan->getVPValueOrAddLiveIn(Store->getValueOperand()), + nullptr /*Mask*/, false /*Consecutive*/, false /*Reverse*/); } else if (GetElementPtrInst *GEP = dyn_cast(Inst)) { NewRecipe = new VPWidenGEPRecipe(GEP, Plan->mapToVPValues(GEP->operands())); @@ -600,9 +602,9 @@ return; LLVMContext &Ctx = SE.getContext(); - auto *BOC = - new VPInstruction(VPInstruction::BranchOnCond, - {Plan.getOrAddExternalDef(ConstantInt::getTrue(Ctx))}); + auto *BOC = new VPInstruction( + VPInstruction::BranchOnCond, + {Plan.getVPValueOrAddLiveIn(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->getVPValueOrAddLiveIn(&*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.getVPValueOrAddLiveIn(ConstantInt::get(Int32, 1)); + VPValue *ExtVPV2 = Plan.getVPValueOrAddLiveIn(ConstantInt::get(Int32, 2)); Args.push_back(ExtVPV1); Args.push_back(ExtVPV2); VPWidenRecipe *WidenR =