[PATCH] D94106: [Local] Treat calls that may not return as being alive (WIP).

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

FWIW, my comment was aimed at inference of `willreturn`, we can have that right now (basically) if we enable a "lightweight" attributor pass.



================
Comment at: llvm/include/llvm/IR/InstrTypes.h:1763
+    return hasFnAttr(Attribute::WillReturn) ||
+           hasFnAttr(Attribute::MustProgress);
+  }
----------------
fhahn wrote:
> jdoerfert wrote:
> > jdoerfert wrote:
> > > nikic wrote:
> > > > This isn't right, MustProgress does not guarantee that the function returns. I'd suggest to take a look at the logic in isGuaranteedToTransferExecutionToSuccessor() (which should be replaced as well).
> > > `willreturn || (mustprogress && readnone)`
> > > 
> > Maybe `readonly` is sufficient given that volatile/atomic read accesses are considered writes, right?
> That's a good point, in general we need to be more careful.
> 
> Though perhaps using `mustprogress` as an initial proxy to enable/disable this altogether would be helpful to limit the fallout at least a little bit during the transition, not here but at the use in `Local.cpp`. instructions there are only considered dead if there are no side-effects at all to start with.
mustprogress alone doesn't imply willreturn, why would we benefit from assuming it would. The deduction of willreturn in the attributor uses mustprogress with D94125.
I'd be more inclined to just look for willreturn and run the deduction early.


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:421
+      return false;
+
   if (!I->mayHaveSideEffects())
----------------
Nit: move the comment before the if or use braces.

`I->getFunction()`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94106