[PATCH] D73311: [Attributor] Use assumed information to determine side-effects

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2704
     if (auto *I = dyn_cast<Instruction>(&V))
-      if (wouldInstructionBeTriviallyDead(I)) {
+      if (isAssumedSideEffectFree(A, I) && !isa<InvokeInst>(I)) {
         A.deleteAfterManifest(*I);
----------------
jdoerfert wrote:
> sstefan1 wrote:
> > Shouldn't `I` always be `AssumedSideEffectFree` at this point?
> That's a good point. I'll check why this is here.
It is because we conflate two things here, (1) dead users and (2) no side-effects. The former allows us to drop the returned values even if we have side-effects (for a function/call). I'll added a comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73311