[PATCH] D50035: [SLC] Expand simplification of pow() for vector types

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:1171-1172
+  // Bail out if simplifying libcalls to pow() is disabled.
+  if (!hasUnaryFloatFn(TLI, Ty, LibFunc_pow, LibFunc_powf, LibFunc_powl))
+    return nullptr;
+
----------------
evandro wrote:
> spatel wrote:
> > evandro wrote:
> > > spatel wrote:
> > > > evandro wrote:
> > > > > spatel wrote:
> > > > > > Why do we need this check? In all of the affected transforms, we're not transforming *to* pow(). Is there a known bug without this check? Given that we have a pow intrinsic, I don't see the problem.
> > > > > Without this check, `pow()` would be simplified even with `-disable-simplify-libcalls` and `pow-3.ll` would fail.
> > > > No, that test still passes (we don't convert) because we check for the availability of sqrt() before trying the transform.
> > > > 
> > > > I suppose this raises a question: if -disable-simplify-libcalls is specified, should that preclude simplifications from LLVM intrinsics that mimic libcalls to other LLVM intrinsics that mimic libcalls?
> > > > ...but that's probably best left to another patch or asked on llvm-dev.
> > > `-disable-simplify-libcalls` disables all functions, including `pow()` itself.  As I understand it, no optimization should then be performed on libcalls.  Again, without these lines, `pow-3.ll` fails.
> > There must be something different between our local machines then. I'm applying this patch on an unmodified trunk r339434, deleting the check for pow(), and running 'make check'. Everything passes. 
> > 
> > And I even stepped through in the debugger to confirm the reason as I stated earlier - the sqrt() check fails, so there is no transform. Can you step through and see how that check is bypassed on your machine?
> You're right.  We're looking at different stages of the tree.  
> 
> Regardless, without this check, `pow()` could still be shrunk below, which I don't think is desired with `-disable-simplify-libcalls`.
No, the shrinking is guarded by TLI->getName(LibFunc_pow) and so that won't happen independent of this patch. I added tests for this at rL339467.


https://reviews.llvm.org/D50035