[PATCH] D28594: NewGVN: Move leaders around properly to ensure we have a canonical dominating leader. Fixes PR 31613.

[Daniel Berlin via Phabricator via llvm-commits]

dberlin marked 6 inline comments as done.
dberlin added inline comments.


================
Comment at: lib/Transforms/Scalar/NewGVN.cpp:1131
+      std::pair<Value *, unsigned> MinDFS = {nullptr, ~0};
+      for (const auto X : OldClass->Members) {
+        auto DFSNum = InstrDFS.lookup(X);
----------------
davide wrote:
> X can probably be passed by reference?
> I'm always in doubt with `auto` (I try to never rely on the implicit referenceness/pointerness of auto and always be explicit to avoid accidental copies)
The members are already value *'s :)



================
Comment at: lib/Transforms/Scalar/NewGVN.cpp:1562-1563
 // congruence classes.  Note that this checking is not perfect, and is currently
-// subject to very rare false negatives. It is only useful for testing/debugging.
+// subject to very rare false negatives. It is only useful for testing and
+// debugging.
 void NewGVN::verifyMemoryCongruency() const {
----------------
davide wrote:
> unrelated I assume?
Yes, i'll revert it.


https://reviews.llvm.org/D28594