[PATCH] D118426: [InstCombine] Remove side effect of replaced constrained intrinsics

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:3373
+    if (CI.getNumUses() == 0)
+      CI.addFnAttr(Attribute::ReadNone);
+    return Res;
----------------
I'm not sure I understand the relevant invariant here.  You're using SimplifyInstruction to assert that the simplified instruction has no side-effects?  That isn't documented anywhere, but I guess it makes sense?  Please confirm and add API documentation explaining this.

If it is safe, we should simplify all calls, not just for ConstrainedFPIntrinsic calls.

Also, you can just erase a call using eraseInstFromFunction; you don't need to mark it readnone and wait for another iteration of instcombine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118426