[PATCH] D97974: [gvn] CSE gc.relocates based on meaning, not spelling

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

A bit of follow up here.   This was reverted in cfe8f8e0f0 <https://reviews.llvm.org/rGcfe8f8e0f010077f5942bce88a2fd331b90ccea7> because the readnone changes exposed a couple of problems.

First, Serguei hit a couple of bugs in the lowering for statepoints which code movement exposed.  These were fixed by D98324 <https://reviews.llvm.org/D98324>, and D98393 <https://reviews.llvm.org/D98393> respectively.  Both issues could in theory have occurred without the readnone change, but became much more common afterwards.

Second, Serguei hit an issue w/LICM sinking gc.relocates and gc.results out of loops, creating phis of token type (which are illegal).  I'm going to explore a fix for this once I more fully understand the issue, but in the meantime, Serguei reverted the original change as we seem to keep hitting more issues.

To be clear, I support Serguei's revert and think it's fully justified given the multiple issues found.  We'll revisit and try to reapply once exposed issues have all been fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97974