[PATCH] D125205: Replace the custom linked list in LeaderTableEntry with TinyPtrVector.

[Owen Anderson via Phabricator via llvm-commits]

resistor added inline comments.


================
Comment at: llvm/include/llvm/Transforms/Scalar/GVN.h:284
+        auto eraseBI = BI--;
+        entry.BB.erase(eraseBI);
       }
----------------
nikic wrote:
> This looks incorrect in multiple ways. If we're at the first instruction, this will step before the begin iterator. And the end iterator is invalidated in either case. I think you need something like this:
> ```
> VI = entry.Val.erase(VI);
> BI = entry.BB.erase(BI);
> VE = entry.Val.end();
> continue;
> ```
Doh!


================
Comment at: llvm/lib/Transforms/Scalar/GVN.cpp:2113
+    }
+  }
+
----------------
nikic wrote:
> Can be just `return is_contained(entry.BB, BB)`?
Done


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125205/new/

https://reviews.llvm.org/D125205