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 @@ -1682,12 +1682,9 @@ /// Holds all the VPConstants created for this VPlan. DenseMap> VPConstants; - /// Holds all the external definitions created for this VPlan. - // TODO: Introduce a specific representation for external definitions in - // VPlan. External definitions must be immutable and hold a pointer to its - // underlying IR that will be used to implement its structural comparison - // (operators '==' and '<'). - SmallPtrSet VPExternalDefs; + /// Holds all the external definitions representing an underlying Value + /// in this VPlan. CFGBuilder ensures these are unique. + SmallVector, 16> VPExternalDefs; /// Represents the backedge taken count of the original loop, for folding /// the tail. @@ -1725,8 +1722,6 @@ delete VPV; if (BackedgeTakenCount) delete BackedgeTakenCount; - for (VPValue *Def : VPExternalDefs) - delete Def; for (VPValue *CBV : VPCBVs) delete CBV; } @@ -1769,10 +1764,10 @@ return UPtr.get(); } - /// Add \p VPVal to the pool of external definitions if it's not already - /// in the pool. - void addExternalDef(VPValue *VPVal) { - VPExternalDefs.insert(VPVal); + /// Create a VPExternalDef for a given Value \p ExtDef. + VPExternalDef *getVPExternalDef(Value *ExtDef) { + VPExternalDefs.emplace_back(new VPExternalDef(ExtDef)); + return VPExternalDefs.back().get(); } /// Add \p CBV to the vector of condition bit values. @@ -1830,6 +1825,9 @@ // consistent with the underlying IR information of each VPConstant. void verifyVPConstants() const; + // Verify that VPExternalDefs are unique in the pool. + void verifyVPExternalDefs() const; + private: /// Add to the given dominator tree the header block and every new basic block /// that was created between it and the latch block, inclusive. 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 @@ -691,6 +691,15 @@ } } +void VPlan::verifyVPExternalDefs() const { + SmallPtrSet ValueSet; + for (const auto &Def : VPExternalDefs) { + const Value *KeyVal = Def->getUnderlyingValue(); + assert(!ValueSet.count(KeyVal) && "Repeated VPExternalDef!"); + ValueSet.insert(KeyVal); + } +} + void VPlan::updateDominatorTree(DominatorTree *DT, BasicBlock *LoopPreHeaderBB, BasicBlock *LoopLatchBB, BasicBlock *LoopExitBB) { @@ -1175,9 +1184,6 @@ void VPSlotTracker::assignSlots(const VPlan &Plan) { - for (const VPValue *V : Plan.VPExternalDefs) - assignSlot(V); - for (auto &E : Plan.Value2VPValue) if (!isa(E.second)) assignSlot(E.second); 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 @@ -184,10 +184,9 @@ // A and B: Create VPValue and add it to the pool of external definitions and // to the Value->VPValue map. - VPValue *NewVPVal = new VPValue(IRVal); - Plan.addExternalDef(NewVPVal); - IRDef2VPValue[IRVal] = NewVPVal; - return NewVPVal; + VPExternalDef *ExtDef = Plan.getVPExternalDef(IRVal); + IRDef2VPValue[IRVal] = ExtDef; + return ExtDef; } // Create new VPInstructions in a VPBasicBlock, given its BasicBlock diff --git a/llvm/lib/Transforms/Vectorize/VPlanValue.h b/llvm/lib/Transforms/Vectorize/VPlanValue.h --- a/llvm/lib/Transforms/Vectorize/VPlanValue.h +++ b/llvm/lib/Transforms/Vectorize/VPlanValue.h @@ -91,6 +91,7 @@ /// type identification. enum { VPConstantSC, + VPExternalDefSC, VPValueSC, VPVInstructionSC, VPVMemoryInstructionSC, @@ -368,6 +369,35 @@ } }; +/// This class augments VPValue with definitions that happen outside of the +/// top region represented in VPlan. Similar to VPConstants and Constants, +/// VPExternalDefs are immutable (once created they never change) and are fully +/// shared by structural equivalence (e.g. i32 %param0 == i32 %param0). They +/// must be created through the VPlan::getVPExternalDef interface, to guarantee +/// that only one instance of each external definition is created. +class VPExternalDef : public VPValue { + // VPlan is currently the context where the pool of VPExternalDefs is held. + friend class VPlan; + +private: + // Construct a VPExternalDef given a Value \p ExtVal. + VPExternalDef(Value *ExtVal) : VPValue(VPValue::VPExternalDefSC, ExtVal) {} + +public: + VPExternalDef(const VPExternalDef &) = delete; + VPExternalDef &operator=(const VPExternalDef &) const = delete; + + void printAsOperand(raw_ostream &OS) const { + assert(UnderlyingVal && "Expected underlying value!"); + UnderlyingVal->printAsOperand(OS); + } + + /// Method to support type inquiry through isa, cast, and dyn_cast. + static inline bool classof(const VPValue *V) { + return V->getVPValueID() == VPExternalDefSC; + } +}; + } // namespace llvm #endif // LLVM_TRANSFORMS_VECTORIZE_VPLAN_VALUE_H diff --git a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp --- a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp +++ b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp @@ -125,6 +125,7 @@ return; Plan.verifyVPConstants(); + Plan.verifyVPExternalDefs(); LLVM_DEBUG(dbgs() << "Verifying VPlan H-CFG.\n"); assert(!TopRegion->getParent() && "VPlan Top Region should have no parent."); 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 @@ -61,6 +61,7 @@ VPInstruction *Idx = cast(&*Iter++); EXPECT_EQ(Instruction::GetElementPtr, Idx->getOpcode()); EXPECT_EQ(2u, Idx->getNumOperands()); + EXPECT_TRUE(isa(Idx->getOperand(0))); EXPECT_EQ(Phi, Idx->getOperand(1)); VPInstruction *Load = cast(&*Iter++); @@ -90,6 +91,7 @@ EXPECT_EQ(Instruction::ICmp, ICmp->getOpcode()); EXPECT_EQ(2u, ICmp->getNumOperands()); EXPECT_EQ(IndvarAdd, ICmp->getOperand(0)); + EXPECT_TRUE(isa(ICmp->getOperand(1))); EXPECT_EQ(VecBB->getCondBit(), ICmp); // Add an external value to check we do not print the list of external values,