[PATCH] D113897: [GVNSink] Make GVNSink resistant against self referencing instructions (PR36954)

[Dawid Jurczak via Phabricator via llvm-commits]

yurai007 added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/GVNSink.cpp:367
+                       return U == (User *)I;
+                     }) != I->users().end();
+    for (auto &U : I->uses()) {
----------------
nikic wrote:
> This checks for a directly self-referential instruction, but the references may also be cyclic across multiple instructions.
> 
> As @xbolva00 mentioned, it may be simpler/cleaner to skip unreachable blocks.
Yes, and that's what I proposed as alternative solution: https://bugs.llvm.org/show_bug.cgi?id=36954#c9 It would save some work and resolve cycles problem. The only drawback I see now is need of giving GVNSink access to DT since we need isReachableFromEntry or analogous machinery for checking reachability. I started with first, simpler approach because it was preferred by @jeroen.dobbelaere, however checking cycles with this idea during lookupOrAdd run seem to be nontrivial. So yes, I can switch to second approach and check where it leads.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113897