[PATCH] D35918: [GVNHoist] Factor out reachability to search for anticipable instructions quickly

Daniel Berlin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 24 12:28:44 PDT 2017


dberlin accepted this revision.
dberlin added a comment.
This revision is now accepted and ready to land.

With these changes, i think it is a good start.



================
Comment at: lib/Transforms/Scalar/GVNHoist.cpp:117
 
-// A map from a pair of VNs to all the instructions with those VNs.
-typedef DenseMap<std::pair<unsigned, unsigned>, SmallVector<Instruction *, 4>>
-    VNtoInsns;
+typedef SmallVectorImpl<CHIArg>::iterator CHIIt;
+typedef DenseMap<BasicBlock *, SmallVector<CHIArg, 2>> OutValuesType;
----------------
All of the uses of this type should just be auto, AFAICT


================
Comment at: lib/Transforms/Scalar/GVNHoist.cpp:557
+      // Find if all the edges have values flowing out of BB.
+      for (const auto &S : TI->successors()) {
+        if (S == Dest)
----------------
This keeps going after it finds anything, despite the fact that the boolean won't change.
I would instead use (you probably need to include STLExtras.h)

auto Found = any_of(TI->successors(), [](const BasicBlock *BB){ return BB == Dest; });




https://reviews.llvm.org/D35918





More information about the llvm-commits mailing list