[PATCH] D64099: [InstCombine] pow(C,x) -> exp2(log2(C)*x)

[Evandro Menezes via Phabricator via llvm-commits]

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

It was actually harder to find a SPEC benchmark that triggered this case.  As a matter of fact, it shows up only in CPU2017's 525.x264_r.  But I can confirm that it works successfully.

Thank you for this patch.



================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:1238
 /// Use exp{,2}(x * y) for pow(exp{,2}(x), y);
 /// exp2(n * x) for pow(2.0 ** n, x); exp10(x) for pow(10.0, x).
 Value *LibCallSimplifier::replacePowWithExp(CallInst *Pow, IRBuilder<> &B) {
----------------
Please, update this comment.


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:1350
 
+  // pow(C,x) -> exp2(log2(C)*x) if C > 0, C != inf, C != NaN
+  if (Pow->hasOneUse() && Pow->hasApproxFunc() && Pow->hasNoNaNs() &&
----------------
Not really necessary to specify the conditions in the comment.


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:1360
+    if (Log) {
+      Value *FMul = B.CreateFMul(Log, Expo, "logmul");
+      if (Pow->doesNotAccessMemory()) {
----------------
`"mul"` was used above to describe products.


================
Comment at: test/Transforms/InstCombine/pow-exp.ll:208
 
+; pow(C,x) -> exp2(log2(C)*x) if C > 0, C != inf, C != NaN
+
----------------
Again, not really necessary to spell out the conditions in the comment.


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

https://reviews.llvm.org/D64099