[PATCH] D75936: Add a Pass to X86 that builds a Condensed CFG for Load Value Injection (LVI) Gadgets [4/6]

[Matthew Riley via Phabricator via llvm-commits]

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