[PATCH] D98728: [LICM] Fix a crash when sinking instructions w/token operands

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 17 14:00:32 PDT 2021


fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:1515
       if (Loop *OLoop = LI->getLoopFor(OInst->getParent()))
-        if (!OLoop->contains(&PN)) {
+        if (!OLoop->contains(&PN) && !Op->getType()->isTokenTy()) {
           PHINode *OpPN =
----------------
reames wrote:
> reames wrote:
> > fhahn wrote:
> > > It might be helpful for future readers of the code if the token-type check would be separate & with a brief comment (perhaps an early continue). I'm fine either way, please feel free to disregard the comment.
> > I played with a couple variations, and everything I came up with was harder to read and lengthened the code a lot.  I ended up slightly rewording the comments describing the actions being done and left it at that.
> Thought about this a bit further, and came up with 31764ea295.  Landed it because it seemed straight forward, but let me know what you think.  I can always revert if you dislike the direction.  
I think that's a great improvement, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98728



More information about the llvm-commits mailing list