[PATCH] D86834: [InstSimplify] Protect against more poison in SimplifyWithOpReplaced (PR47322)

[Nikita Popov via Phabricator via llvm-commits]

nikic added a subscriber: fhahn.
nikic added inline comments.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:3799
     //       by dropping the flags. Remove this fold to save compile-time?
-    if (isa<OverflowingBinaryOperator>(B))
-      if (Q.IIQ.hasNoSignedWrap(B) || Q.IIQ.hasNoUnsignedWrap(B))
-        return nullptr;
-    if (isa<PossiblyExactOperator>(B) && Q.IIQ.isExact(B))
+    if (canCreatePoison(cast<Operator>(I)))
       return nullptr;
----------------
aqjune wrote:
> This diff has a side effect when IIQ's UseInstrInfo is false (and I think it is okay because this change leads to less potential poison-related problems, but) when is the flag set to false?
Yes, this is intentional. UseInstrInfo=false should result in less folds, not more. It may be even more correct to check for `|| !Q.IIQ.UseInstrInfo` here, i.e. make the conservative assumption that the instruction does have poison flags, if we are not allowed to inspect the actual poison flags on the instruction.

Maybe @fhahn can tell whether this is necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86834