[PATCH] fold: sqrt(x * x * y) -> fabs(x) * sqrt(y)

[hfinkel at anl.gov]

================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:1270
@@ +1269,3 @@
+  if (Instruction *I = dyn_cast<Instruction>(Op)) {
+    if (I->getOpcode() == Instruction::FMul && I->hasUnsafeAlgebra()) {
+      // We're looking for a repeated factor in a multiplication tree,
----------------
spatel wrote:
> hfinkel wrote:
> > Hrmm... is this right, or do we need to check the function attribute here? I'm not sure that the argument having the unsafe-algebra flag means that we can change its use in a non-strict way.
> I think this goes back to the discussion of whether 'fast' is infectious in http://reviews.llvm.org/D5584. :)
> In that case, we optimized away a sqrt call even though it wasn't explicitly marked 'fast'. This is a similar transform. We've just reversed the order of the fmul and sqrt.
> 
> I thought I saw some other precedence for just using the instruction-level flags, but I'm not finding it now. There's an optimization for log2() in InstCombineMulDivRem that checks whether the log2 intrinsic hasUnsafeAlgebra...I don't know how that is even specified in IR. I thought the IR fast-math flags only apply to fmul, fdiv, fadd, fsub, and frem? At least, that's what the LangRef says. FWIW, that log2 optimization doesn't appear to ever trigger, and I don't see any test case for it (r169025).
> In that case, we optimized away a sqrt call even though it wasn't explicitly marked 'fast'. This is a similar transform. We've just reversed the order of the fmul and sqrt.

Right, but in that case we were optimizing the fmul, and it was the "outer" operation, and it had fast. In this case, the sqrt is the outer operation, and we need to check its equivalent.

Generally speaking, fast-math flags cannot infect users (only operands), except for some result assumptions, because of the inlining use case.

> FWIW, that log2 optimization doesn't appear to ever trigger, and I don't see any test case for it (r169025).

Seems like this is a bug (I would not object to enhancing the IR to support fast-math flags on intrinsics, but as it stands, it is a bug).

http://reviews.llvm.org/D5787