[PATCH] D64099: [InstCombine] pow(C,x) -> exp(log(C)*x)

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

I'm a little wary about fold the case where doesNotAccessMemory is false, but I guess it's likely okay.

I'm guessing it doesn't matter whether you use exp or exp2 here?  I guess the performance is probably similar, but it isn't obvious to me.

Should we specifically have test coverage for transforming `pow(10, x)` to exp?



================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:1351
+  // pow(C,x) -> exp(log(C)*x) if C > 0, C != inf, C != NaN
+  if (Pow->hasOneUse() && Pow->isFast() && BaseF->isNormal() &&
+      !BaseF->isNegative()) {
----------------
Please don't check "isFast()", instead, check the components you actually need for this.  You clearly need afn; not sure if you need anything else to handle cases where the exponent is zero/inf/nan.


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:1354
+    Value *LogC =
+        emitUnaryFloatFnCall(ConstantFP::get(Ty, *BaseF), "log", B, Attrs);
+    Value *FMul = B.CreateFMul(LogC, Expo, "logmul");
----------------
I'd rather explicitly fold the "log" here, so we know it actually happens; the constant folding code will not fold it in all cases.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64099