[PATCH] D32664: [LoopUnswitch] Don't remove instructions with side effects after folding them

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc accepted this revision.
chandlerc added a comment.
This revision is now accepted and ready to land.

This seems correct and safe. Happy for you to land as-is, or to try a different approach if you want. I don't think you're making anything worse, that's for sure. =D



================
Comment at: lib/Transforms/Scalar/LoopUnswitch.cpp:1278-1279
   I->replaceAllUsesWith(V);
-  I->eraseFromParent();
+  if (!I->mayHaveSideEffects())
+    I->eraseFromParent();
   ++NumSimplify;
----------------
davide wrote:
> davide wrote:
> > Alternatively this could use `isInstructionTriviallyDead`. At that point `LoopUnswitch` should require `TLI` I guess and we should thread it properly in the pass. 
> > I don't mind doing that, FWIW, if you prefer and/or it's better.
> Nevermind, I was wrong, it doesn't require `TLI`. 
FWIW, I would keep this surgical given the work I'm doing on LoopUnswitch. But I don't feel strongly either way.


https://reviews.llvm.org/D32664