mattdr marked an inline comment as done.
mattdr added inline comments.
================
Comment at: llvm/lib/Target/X86/ImmutableGraph.h:366
+ std::vector<size_type> TrimmedNodes(TrimNodes.size());
+ for (size_type I = 0; I < TrimNodes.size(); ++I) {
+ TrimmedNodes[I] = TrimmedNodesSoFar;
----------------
craig.topper wrote:
> mattdr wrote:
> > Two comments here would make the code significantly easier to understand:
> > 1. Note that we're using `.size()` here rather than `.count()`, so we're
> > actually iterating over *all* Node indices, not just the ones to be trimmed
> > 2. The `TrimmedNodes` vector maps indices in the original NodeSet to the
> > number of `Node`s before that index that have been trimmed by that index,
> > to allow later code to map elements to their new position in a dense array
> > with the trimmed items removed
> I've stopped using TrimmedNodes.size() and TrimEdges.size() in favor of the
> size methods from the graph which should make things more obvious.
>
> I renamed TrimmedNodes to RemappedNodeIndex and stored the new index rather
> than the adjustment needed. I'm also changed it to walk nodes instead of
> indices so we don't have to translate to Node to make the contains call.
>
> I also removed the NewNumEdges count_if and the if statement around the edge
> loop from the loop below. I don't think that provided any value and just
> complicated the code.
Many thanks! These changes make the code much more accessible.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75936/new/
https://reviews.llvm.org/D75936
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
