[PATCH] D87877: [InstCombine] Fix errno bug in pow expansion to sqrt

[Sanjay Patel via Phabricator via llvm-commits]

spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:1644
+  if (!Pow->doesNotAccessMemory() && !Pow->hasNoInfs() &&
+      !isKnownNeverInfinity(Base, TLI))
+    return nullptr;
----------------
hubert.reinterpretcast wrote:
> @spatel, I've added the `isKnownNeverInfinity` check here as suggested in your comment below. I am not entirely sure whether it is quite effective though. It seems this query may happening "too early".
> 
> Example case I tried (with `opt -instcombine`):
> ```
> define double @pow_libcall_half_no_FMF_base_ninf(i32 %x) {
>   %conv = sitofp i32 %x to double
>   %pow = call double @pow(double %conv, double 5.0e-01)
>   ret double %pow
> }
> ```
This is yet another bug independent of this patch:

```
$ opt -instsimplify inf.ll -S
define i1 @src(i32 %x) {
  %conv = sitofp i32 %x to double
  %r = fcmp oeq double %conv, 0x7FF0000000000000
  ret i1 %r
}
$ opt -instcombine inf.ll -S
define i1 @src(i32 %x) {
  ret i1 false
}

```
https://alive2.llvm.org/ce/z/a54zEx
So instcombine knows that transform, but ValueTracking/instsimplify do not. I'll put that on my TODO list. If you change your example to use "uitofp" it should work as expected.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87877