[PATCH] D149587: InstSimplify: Simplifications for ldexp

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Analysis/ConstantFolding.cpp:2706
 
-      if (IntrinsicID == Intrinsic::amdgcn_ldexp) {
+      if (IntrinsicID == Intrinsic::ldexp) {
         // FIXME: Should flush denorms depending on FP mode, but that's ignored
----------------
foad wrote:
> Should keep support for the old intrinsic while it still exists.
There's no reason to aim for performance compatibility with something trivially replaceable. It's already dropped in D149589


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:6034
+
+  // ldexp(x, 0) -> x
+  if (match(Op1, PatternMatch::m_ZeroInt()))
----------------
kpn wrote:
> foad wrote:
> > Why is this not strictfp-safe?
> It would pass through an SNaN instead of quieting it I expect.
Yes, needs to quiet/canonicalize which isn't guaranteed for non-constrained ops


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

https://reviews.llvm.org/D149587