<div dir="ltr">That check was to catch where Idx == AdjEdgeIds.size() - 1 in order to avoid a redundant swap. That's not actually worth the effort though.<div><br></div><div>I've simplified this in r204857.</div><div>
<br></div><div>- Lang.</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Mar 26, 2014 at 12:17 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Wed, Mar 26, 2014 at 11:58 AM, Lang Hames <<a href="mailto:lhames@gmail.com">lhames@gmail.com</a>> wrote:<br>
> Author: lhames<br>
> Date: Wed Mar 26 13:58:00 2014<br>
> New Revision: 204841<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=204841&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=204841&view=rev</a><br>
> Log:<br>
> Change the PBQP graph adjacency list structure from std::set to std::vector.<br>
><br>
> The edge data structure (EdgeEntry) now holds the indices of its entries in the<br>
> adjacency lists of the nodes it connects. This trades a little ugliness for<br>
> faster insertion/removal, which is now O(1) with a cheap constant factor. All<br>
> of this is implementation detail within the PBQP graph, the external API remains<br>
> unchanged.<br>
><br>
> Individual register allocations are likely to change, since the adjacency lists<br>
> will now be ordered differently (or rather, will now be unordered). This<br>
> shouldn't affect the average quality of allocations however.<br>
><br>
><br>
> Modified:<br>
> llvm/trunk/include/llvm/CodeGen/PBQP/Graph.h<br>
><br>
> Modified: llvm/trunk/include/llvm/CodeGen/PBQP/Graph.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/PBQP/Graph.h?rev=204841&r1=204840&r2=204841&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/PBQP/Graph.h?rev=204841&r1=204840&r2=204841&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/include/llvm/CodeGen/PBQP/Graph.h (original)<br>
> +++ llvm/trunk/include/llvm/CodeGen/PBQP/Graph.h Wed Mar 26 13:58:00 2014<br>
> @@ -51,29 +51,115 @@ namespace PBQP {<br>
><br>
> class NodeEntry {<br>
> public:<br>
> - typedef std::set<NodeId> AdjEdgeList;<br>
> + typedef std::vector<EdgeId> AdjEdgeList;<br>
> + typedef AdjEdgeList::size_type AdjEdgeIdx;<br>
> typedef AdjEdgeList::const_iterator AdjEdgeItr;<br>
> +<br>
> + static AdjEdgeIdx getInvalidAdjEdgeIdx() {<br>
> + return std::numeric_limits<AdjEdgeIdx>::max();<br>
> + }<br>
> +<br>
> NodeEntry(VectorPtr Costs) : Costs(Costs) {}<br>
><br>
> + AdjEdgeIdx addAdjEdgeId(EdgeId EId) {<br>
> + AdjEdgeIdx Idx = AdjEdgeIds.size();<br>
> + AdjEdgeIds.push_back(EId);<br>
> + return Idx;<br>
> + }<br>
> +<br>
> + // If a swap is performed, returns the new EdgeId that must be<br>
> + // updated, otherwise returns invalidEdgeId().<br>
> + EdgeId removeAdjEdgeId(AdjEdgeIdx Idx) {<br>
> + EdgeId EIdToUpdate = Graph::invalidEdgeId();<br>
> + if (Idx < AdjEdgeIds.size() - 1) {<br>
<br>
</div></div>Hrm - why isn't this ^ an assert? When do you try to remove edges that<br>
aren't in the edge list?<br>
<div class="HOEnZb"><div class="h5"><br>
> + std::swap(AdjEdgeIds[Idx], AdjEdgeIds.back());<br>
> + EIdToUpdate = AdjEdgeIds[Idx];<br>
> + }<br>
> + AdjEdgeIds.pop_back();<br>
> + return EIdToUpdate;<br>
> + }<br>
> +<br>
> + const AdjEdgeList& getAdjEdgeIds() const { return AdjEdgeIds; }<br>
> +<br>
> VectorPtr Costs;<br>
> NodeMetadata Metadata;<br>
> + private:<br>
> AdjEdgeList AdjEdgeIds;<br>
> };<br>
><br>
> class EdgeEntry {<br>
> public:<br>
> EdgeEntry(NodeId N1Id, NodeId N2Id, MatrixPtr Costs)<br>
> - : Costs(Costs), N1Id(N1Id), N2Id(N2Id) {}<br>
> + : Costs(Costs) {<br>
> + NIds[0] = N1Id;<br>
> + NIds[1] = N2Id;<br>
> + ThisEdgeAdjIdxs[0] = NodeEntry::getInvalidAdjEdgeIdx();<br>
> + ThisEdgeAdjIdxs[1] = NodeEntry::getInvalidAdjEdgeIdx();<br>
> + }<br>
> +<br>
> void invalidate() {<br>
> - N1Id = N2Id = Graph::invalidNodeId();<br>
> + NIds[0] = NIds[1] = Graph::invalidNodeId();<br>
> + ThisEdgeAdjIdxs[0] = ThisEdgeAdjIdxs[1] =<br>
> + NodeEntry::getInvalidAdjEdgeIdx();<br>
> Costs = nullptr;<br>
> }<br>
> - NodeId getN1Id() const { return N1Id; }<br>
> - NodeId getN2Id() const { return N2Id; }<br>
> +<br>
> + void connectToN(Graph &G, EdgeId ThisEdgeId, unsigned NIdx) {<br>
> + assert(ThisEdgeAdjIdxs[NIdx] == NodeEntry::getInvalidAdjEdgeIdx() &&<br>
> + "Edge already connected to NIds[NIdx].");<br>
> + NodeEntry &N = G.getNode(NIds[NIdx]);<br>
> + ThisEdgeAdjIdxs[NIdx] = N.addAdjEdgeId(ThisEdgeId);<br>
> + }<br>
> +<br>
> + void connectTo(Graph &G, EdgeId ThisEdgeId, NodeId NId) {<br>
> + if (NId == NIds[0])<br>
> + connectToN(G, ThisEdgeId, 0);<br>
> + else {<br>
> + assert(NId == NIds[1] && "Edge does not connect NId.");<br>
> + connectToN(G, ThisEdgeId, 1);<br>
> + }<br>
> + }<br>
> +<br>
> + void connect(Graph &G, EdgeId ThisEdgeId) {<br>
> + connectToN(G, ThisEdgeId, 0);<br>
> + connectToN(G, ThisEdgeId, 1);<br>
> + }<br>
> +<br>
> + void updateAdjEdgeIdx(NodeId NId, typename NodeEntry::AdjEdgeIdx NewIdx) {<br>
> + if (NId == NIds[0])<br>
> + ThisEdgeAdjIdxs[0] = NewIdx;<br>
> + else {<br>
> + assert(NId == NIds[1] && "Edge not connected to NId");<br>
> + ThisEdgeAdjIdxs[1] = NewIdx;<br>
> + }<br>
> + }<br>
> +<br>
> + void disconnectFromN(Graph &G, unsigned NIdx) {<br>
> + assert(ThisEdgeAdjIdxs[NIdx] != NodeEntry::getInvalidAdjEdgeIdx() &&<br>
> + "Edge not connected to NIds[NIdx].");<br>
> + NodeEntry &N = G.getNode(NIds[NIdx]);<br>
> + EdgeId EIdToUpdate = N.removeAdjEdgeId(ThisEdgeAdjIdxs[NIdx]);<br>
> + if (EIdToUpdate != Graph::invalidEdgeId())<br>
> + G.getEdge(EIdToUpdate).updateAdjEdgeIdx(NIds[NIdx], ThisEdgeAdjIdxs[NIdx]);<br>
> + ThisEdgeAdjIdxs[NIdx] = NodeEntry::getInvalidAdjEdgeIdx();<br>
> + }<br>
> +<br>
> + void disconnectFrom(Graph &G, NodeId NId) {<br>
> + if (NId == NIds[0])<br>
> + disconnectFromN(G, 0);<br>
> + else {<br>
> + assert(NId == NIds[1] && "Edge does not connect NId");<br>
> + disconnectFromN(G, 1);<br>
> + }<br>
> + }<br>
> +<br>
> + NodeId getN1Id() const { return NIds[0]; }<br>
> + NodeId getN2Id() const { return NIds[1]; }<br>
> MatrixPtr Costs;<br>
> EdgeMetadata Metadata;<br>
> private:<br>
> - NodeId N1Id, N2Id;<br>
> + NodeId NIds[2];<br>
> + typename NodeEntry::AdjEdgeIdx ThisEdgeAdjIdxs[2];<br>
> };<br>
><br>
> // ----- MEMBERS -----<br>
> @@ -134,8 +220,8 @@ namespace PBQP {<br>
> (N2.Costs->getLength() == NE.Costs->getCols()) &&<br>
> "Edge cost dimensions do not match node costs dimensions.");<br>
><br>
> - N1.AdjEdgeIds.insert(EId);<br>
> - N2.AdjEdgeIds.insert(EId);<br>
> + // Add the edge to the adjacency sets of its nodes.<br>
> + NE.connect(*this, EId);<br>
> return EId;<br>
> }<br>
><br>
> @@ -228,14 +314,14 @@ namespace PBQP {<br>
> public:<br>
> AdjEdgeIdSet(const NodeEntry &NE) : NE(NE) { }<br>
> typename NodeEntry::AdjEdgeItr begin() const {<br>
> - return NE.AdjEdgeIds.begin();<br>
> + return NE.getAdjEdgeIds().begin();<br>
> }<br>
> typename NodeEntry::AdjEdgeItr end() const {<br>
> - return NE.AdjEdgeIds.end();<br>
> + return NE.getAdjEdgeIds().end();<br>
> }<br>
> - bool empty() const { return NE.AdjEdges.empty(); }<br>
> + bool empty() const { return NE.getAdjEdgeIds().empty(); }<br>
> typename NodeEntry::AdjEdgeList::size_type size() const {<br>
> - return NE.AdjEdgeIds.size();<br>
> + return NE.getAdjEdgeIds().size();<br>
> }<br>
> private:<br>
> const NodeEntry &NE;<br>
> @@ -336,7 +422,7 @@ namespace PBQP {<br>
> }<br>
><br>
> typename NodeEntry::AdjEdgeList::size_type getNodeDegree(NodeId NId) const {<br>
> - return getNode(NId).AdjEdgeIds.size();<br>
> + return getNode(NId).getAdjEdgeIds().size();<br>
> }<br>
><br>
> /// \brief Set an edge's cost matrix.<br>
> @@ -459,8 +545,9 @@ namespace PBQP {<br>
> void disconnectEdge(EdgeId EId, NodeId NId) {<br>
> if (Solver)<br>
> Solver->handleDisconnectEdge(EId, NId);<br>
> - NodeEntry &N = getNode(NId);<br>
> - N.AdjEdgeIds.erase(EId);<br>
> +<br>
> + EdgeEntry &E = getEdge(EId);<br>
> + E.disconnectFrom(*this, NId);<br>
> }<br>
><br>
> /// \brief Convenience method to disconnect all neighbours from the given<br>
> @@ -475,8 +562,8 @@ namespace PBQP {<br>
> /// Adds an edge that had been previously disconnected back into the<br>
> /// adjacency set of the nodes that the edge connects.<br>
> void reconnectEdge(EdgeId EId, NodeId NId) {<br>
> - NodeEntry &N = getNode(NId);<br>
> - N.addAdjEdge(EId);<br>
> + EdgeEntry &E = getEdge(EId);<br>
> + E.connectTo(*this, EId, NId);<br>
> if (Solver)<br>
> Solver->handleReconnectEdge(EId, NId);<br>
> }<br>
> @@ -487,10 +574,7 @@ namespace PBQP {<br>
> if (Solver)<br>
> Solver->handleRemoveEdge(EId);<br>
> EdgeEntry &E = getEdge(EId);<br>
> - NodeEntry &N1 = getNode(E.getNode1());<br>
> - NodeEntry &N2 = getNode(E.getNode2());<br>
> - N1.removeEdge(EId);<br>
> - N2.removeEdge(EId);<br>
> + E.disconnect();<br>
> FreeEdgeIds.push_back(EId);<br>
> Edges[EId].invalidate();<br>
> }<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>