[llvm] r206857 - blockfreq: Use pointers to loops instead of an index
David Blaikie
dblaikie at gmail.com
Mon Apr 21 21:42:45 PDT 2014
On Mon, Apr 21, 2014 at 9:39 PM, David Blaikie <dblaikie at gmail.com> wrote:
> On Mon, Apr 21, 2014 at 8:31 PM, Duncan P. N. Exon Smith
> <dexonsmith at apple.com> wrote:
>> Author: dexonsmith
>> Date: Mon Apr 21 22:31:37 2014
>> New Revision: 206857
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=206857&view=rev
>> Log:
>> blockfreq: Use pointers to loops instead of an index
>>
>> Store pointers directly to loops inside the nodes. This could have been
>> done without changing the type stored in `std::vector<>`. However,
>> rather than computing the number of loops before constructing them
>> (which `LoopInfo` doesn't provide directly), I've switched to a
>> `vector<unique_ptr<LoopData>>`.
>
> Why not use a std::list<LoopData>? If you're going to incur a heap
> allocation for every node anyway, this at least hides it as an
> implementation detail (& you can include a comment explaining that
> you're taking pointers to these things and so you need the iterator
> (non)invalidation semantics of list over vector)
>
> I've made a change from vector<(owning)T*> to list<T> in one or two
> places today for just this reason.
(or perhaps a std::deque - which I hadn't considered before, but works
since I (& you, I think) don't need validity of pointers over removal
operations - just additions at the end)
>
>>
>> This adds some heap overhead, but the number of loops is typically
>> small.
>>
>> Modified:
>> llvm/trunk/include/llvm/Analysis/BlockFrequencyInfoImpl.h
>> llvm/trunk/lib/Analysis/BlockFrequencyInfoImpl.cpp
>>
>> Modified: llvm/trunk/include/llvm/Analysis/BlockFrequencyInfoImpl.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/BlockFrequencyInfoImpl.h?rev=206857&r1=206856&r2=206857&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Analysis/BlockFrequencyInfoImpl.h (original)
>> +++ llvm/trunk/include/llvm/Analysis/BlockFrequencyInfoImpl.h Mon Apr 21 22:31:37 2014
>> @@ -945,28 +945,30 @@ public:
>> typedef SmallVector<std::pair<BlockNode, BlockMass>, 4> ExitMap;
>> typedef SmallVector<BlockNode, 4> MemberList;
>> BlockNode Header; ///< Header.
>> + bool IsPackaged; ///< Whether this has been packaged.
>> ExitMap Exits; ///< Successor edges (and weights).
>> MemberList Members; ///< Members of the loop.
>> BlockMass BackedgeMass; ///< Mass returned to loop header.
>> BlockMass Mass;
>> Float Scale;
>>
>> - LoopData(const BlockNode &Header) : Header(Header) {}
>> + LoopData(const BlockNode &Header) : Header(Header), IsPackaged(false) {}
>> };
>>
>> /// \brief Index of loop information.
>> struct WorkingData {
>> + LoopData *Loop; ///< The loop this block is the header of.
>> BlockNode ContainingLoop; ///< The block whose loop this block is inside.
>> - uint32_t LoopIndex; ///< Index into PackagedLoops.
>> bool IsPackaged; ///< Has ContainingLoop been packaged up?
>> - bool IsAPackage; ///< Has this block's loop been packaged up?
>> BlockMass Mass; ///< Mass distribution from the entry block.
>>
>> - WorkingData()
>> - : LoopIndex(UINT32_MAX), IsPackaged(false), IsAPackage(false) {}
>> + WorkingData() : Loop(nullptr), IsPackaged(false) {}
>>
>> bool hasLoopHeader() const { return ContainingLoop.isValid(); }
>> - bool isLoopHeader() const { return LoopIndex != UINT32_MAX; }
>> + bool isLoopHeader() const { return Loop; }
>> +
>> + /// \brief Has this block's loop been packaged up?
>> + bool isAPackage() const { return Loop && Loop->IsPackaged; }
>> };
>>
>> /// \brief Unscaled probability weight.
>> @@ -1040,7 +1042,7 @@ public:
>> std::vector<WorkingData> Working;
>>
>> /// \brief Indexed information about packaged loops.
>> - std::vector<LoopData> PackagedLoops;
>> + std::vector<std::unique_ptr<LoopData>> PackagedLoops;
>>
>> /// \brief Add all edges out of a packaged loop to the distribution.
>> ///
>> @@ -1061,9 +1063,8 @@ public:
>>
>> LoopData &getLoopPackage(const BlockNode &Head) {
>> assert(Head.Index < Working.size());
>> - size_t Index = Working[Head.Index].LoopIndex;
>> - assert(Index < PackagedLoops.size());
>> - return PackagedLoops[Index];
>> + assert(Working[Head.Index].Loop != nullptr);
>> + return *Working[Head.Index].Loop;
>> }
>>
>> /// \brief Distribute mass according to a distribution.
>> @@ -1428,8 +1429,8 @@ template <class BT> void BlockFrequencyI
>> BlockNode Header = getNode(Loop->getHeader());
>> assert(Header.isValid());
>>
>> - Working[Header.Index].LoopIndex = PackagedLoops.size();
>> - PackagedLoops.emplace_back(Header);
>> + PackagedLoops.emplace_back(new LoopData(Header));
>> + Working[Header.Index].Loop = PackagedLoops.back().get();
>> DEBUG(dbgs() << " - loop = " << getBlockName(Header) << "\n");
>> }
>>
>> @@ -1452,7 +1453,7 @@ template <class BT> void BlockFrequencyI
>> assert(HeaderData.isLoopHeader());
>>
>> Working[Index].ContainingLoop = Header;
>> - PackagedLoops[HeaderData.LoopIndex].Members.push_back(Index);
>> + HeaderData.Loop->Members.push_back(Index);
>> DEBUG(dbgs() << " - loop = " << getBlockName(Header)
>> << ": member = " << getBlockName(Index) << "\n");
>> }
>> @@ -1460,7 +1461,7 @@ template <class BT> void BlockFrequencyI
>>
>> template <class BT> void BlockFrequencyInfoImpl<BT>::computeMassInLoops() {
>> // Visit loops with the deepest first, and the top-level loops last.
>> - for (auto L = PackagedLoops.rbegin(), LE = PackagedLoops.rend(); L != LE; ++L)
>> + for (const auto &L : make_range(PackagedLoops.rbegin(), PackagedLoops.rend()))
>> computeMassInLoop(L->Header);
>> }
>>
>>
>> Modified: llvm/trunk/lib/Analysis/BlockFrequencyInfoImpl.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/BlockFrequencyInfoImpl.cpp?rev=206857&r1=206856&r2=206857&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Analysis/BlockFrequencyInfoImpl.cpp (original)
>> +++ llvm/trunk/lib/Analysis/BlockFrequencyInfoImpl.cpp Mon Apr 21 22:31:37 2014
>> @@ -602,7 +602,7 @@ void BlockFrequencyInfoImplBase::clear()
>> // does not actually clear heap storage.
>> std::vector<FrequencyData>().swap(Freqs);
>> std::vector<WorkingData>().swap(Working);
>> - std::vector<LoopData>().swap(PackagedLoops);
>> + std::vector<std::unique_ptr<LoopData>>().swap(PackagedLoops);
>> }
>>
>> /// \brief Clear all memory not needed downstream.
>> @@ -646,7 +646,7 @@ static BlockMass &getPackageMass(BlockFr
>> const BlockNode &Node) {
>> assert(Node.isValid());
>> assert(!BFI.Working[Node.Index].IsPackaged);
>> - if (!BFI.Working[Node.Index].IsAPackage)
>> + if (!BFI.Working[Node.Index].isAPackage())
>> return BFI.Working[Node.Index].Mass;
>>
>> return BFI.getLoopPackage(Node).Mass;
>> @@ -744,8 +744,9 @@ void BlockFrequencyInfoImplBase::compute
>> /// \brief Package up a loop.
>> void BlockFrequencyInfoImplBase::packageLoop(const BlockNode &LoopHead) {
>> DEBUG(dbgs() << "packaging-loop: " << getBlockName(LoopHead) << "\n");
>> - Working[LoopHead.Index].IsAPackage = true;
>> - for (const BlockNode &M : getLoopPackage(LoopHead).Members) {
>> + auto &PackagedLoop = getLoopPackage(LoopHead);
>> + PackagedLoop.IsPackaged = true;
>> + for (const BlockNode &M : PackagedLoop.Members) {
>> DEBUG(dbgs() << " - node: " << getBlockName(M.Index) << "\n");
>> Working[M.Index].IsPackaged = true;
>> }
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list