[PATCH] D126190: [InstCombine] Add combine for fcmp sqrt(x),C --> fcmp x,C*C

[Bradley Smith via Phabricator via llvm-commits]

bsmith added a comment.

In D126190#3531596 <https://reviews.llvm.org/D126190#3531596>, @xbolva00 wrote:

> Is this “safe” even with fast math for all predicates?

With the fast math flags requirement moved to the sqrt itself, do you still see a problem here?



================
Comment at: llvm/test/Transforms/InstCombine/fcmp.ll:1234
+  %sqrt = call double @llvm.sqrt.f64(double %v)
+  %cmp = fcmp fast ogt double %sqrt, -2.000000e+00
+  ret i1 %cmp
----------------
spatel wrote:
> spatel wrote:
> > Can we handle this kind of example as a preliminary patch? There's no fast-math required if we are checking if the result of sqrt is or is not negative (but we should test several predicates to verify that we have the correct behavior with NAN):
> > https://alive2.llvm.org/ce/z/EeYGLt
> > https://alive2.llvm.org/ce/z/X7qsg2
> Note that we should already reduce some compares via:
> https://github.com/llvm/llvm-project/blob/main/llvm/lib/Analysis/InstructionSimplify.cpp#L3995
That optimization is orthogonal to what we are doing here, additionally I don't think I have the floating-point expertise in order to get the NaN cases in such a change right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126190