[llvm] r204857 - Simplify PBQP graph removeAdjEdgeId implementation.
Lang Hames
lhames at gmail.com
Wed Mar 26 15:16:38 PDT 2014
Correct on all points. Well - I made the last cosmetic fixes - I think it's
your turn to make these. ;)
It's about time you got your name on the blame list for this thing anyway.
- Lang.
On Wed, Mar 26, 2014 at 2:55 PM, David Blaikie <dblaikie at gmail.com> wrote:
> On Wed, Mar 26, 2014 at 2:21 PM, Lang Hames <lhames at gmail.com> wrote:
> > Author: lhames
> > Date: Wed Mar 26 16:21:53 2014
> > New Revision: 204857
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=204857&view=rev
> > Log:
> > Simplify PBQP graph removeAdjEdgeId implementation.
> >
> > Modified:
> > llvm/trunk/include/llvm/CodeGen/PBQP/Graph.h
> >
> > Modified: llvm/trunk/include/llvm/CodeGen/PBQP/Graph.h
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/PBQP/Graph.h?rev=204857&r1=204856&r2=204857&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/include/llvm/CodeGen/PBQP/Graph.h (original)
> > +++ llvm/trunk/include/llvm/CodeGen/PBQP/Graph.h Wed Mar 26 16:21:53 2014
> > @@ -67,16 +67,16 @@ namespace PBQP {
> > return Idx;
> > }
> >
> > - // If a swap is performed, returns the new EdgeId that must be
> > - // updated, otherwise returns invalidEdgeId().
> > - EdgeId removeAdjEdgeId(AdjEdgeIdx Idx) {
> > - EdgeId EIdToUpdate = Graph::invalidEdgeId();
> > - if (Idx < AdjEdgeIds.size() - 1) {
> > - std::swap(AdjEdgeIds[Idx], AdjEdgeIds.back());
> > - EIdToUpdate = AdjEdgeIds[Idx];
> > - }
> > + void removeAdjEdgeId(Graph &G, NodeId ThisNId, AdjEdgeIdx Idx) {
> > + // Swap-and-pop for fast removal.
> > + // 1) Update the adj index of the edge currently at back().
> > + // 2) Swap Edge at Idx with back().
> > + // 3) pop_back()
> > + // If Idx == size() - 1 then the updateAdjEdgeIdx and swap are
> > + // redundant, but both operations are cheap.
> > + G.getEdge(AdjEdgeIds.back()).updateAdjEdgeIdx(ThisNId, Idx);
>
> Is there more to this "updateAdjEdgeIdx" function or is it just a
> trivial (indexed/mapped) setter? "update" rather than "set" makes me
> wonder what work it's doing...
>
> > + std::swap(AdjEdgeIds[Idx], AdjEdgeIds.back());
>
> Since you're about to pop the back anyway, this could just be
> AdjEdgeIds[Idx] = AdjEdgeIds.back(); right? (if they were movable, you
> could add a std::move() around the RHS, but they're just indexes,
> right?)
>
> (& comment 2 would be "move back() down to fill the hole at Idx" or
> something like that)
>
> > AdjEdgeIds.pop_back();
> > - return EIdToUpdate;
> > }
> >
> > const AdjEdgeList& getAdjEdgeIds() const { return AdjEdgeIds; }
> > @@ -138,9 +138,7 @@ namespace PBQP {
> > assert(ThisEdgeAdjIdxs[NIdx] !=
> NodeEntry::getInvalidAdjEdgeIdx() &&
> > "Edge not connected to NIds[NIdx].");
> > NodeEntry &N = G.getNode(NIds[NIdx]);
> > - EdgeId EIdToUpdate = N.removeAdjEdgeId(ThisEdgeAdjIdxs[NIdx]);
> > - if (EIdToUpdate != Graph::invalidEdgeId())
> > - G.getEdge(EIdToUpdate).updateAdjEdgeIdx(NIds[NIdx],
> ThisEdgeAdjIdxs[NIdx]);
> > + N.removeAdjEdgeId(G, NIds[NIdx], ThisEdgeAdjIdxs[NIdx]);
> > ThisEdgeAdjIdxs[NIdx] = NodeEntry::getInvalidAdjEdgeIdx();
> > }
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140326/309c3e8e/attachment.html>
More information about the llvm-commits
mailing list