[PATCH] D13994: [SimplifyLibCalls] Optimization for pow(x, n) where n is some constant

[Mandeep Singh Grang via llvm-commits]

mgrang added inline comments.

================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:1068-1069
@@ +1067,4 @@
+
+  Value *AddChain[33];
+  AddChain[1]  = pow1;
+  AddChain[2]  = B.CreateFMul(pow1, pow1);
----------------
majnemer wrote:
> Perhaps we should assert that `Exp != 0` ?
I will fix this.

================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:1088-1101
@@ +1087,16 @@
+  AddChain[19] = B.CreateFMul(AddChain[1],  AddChain[18]);
+  AddChain[20] = B.CreateFMul(AddChain[10], AddChain[10]);
+  AddChain[21] = B.CreateFMul(AddChain[6],  AddChain[15]);
+  AddChain[22] = B.CreateFMul(AddChain[11], AddChain[11]);
+  AddChain[23] = B.CreateFMul(AddChain[3],  AddChain[20]);
+  AddChain[24] = B.CreateFMul(AddChain[12], AddChain[12]);
+  AddChain[25] = B.CreateFMul(AddChain[8],  AddChain[17]);
+  AddChain[26] = B.CreateFMul(AddChain[13], AddChain[13]);
+  AddChain[27] = B.CreateFMul(AddChain[3],  AddChain[24]);
+  AddChain[28] = B.CreateFMul(AddChain[14], AddChain[14]);
+  AddChain[29] = B.CreateFMul(AddChain[4],  AddChain[25]);
+  AddChain[30] = B.CreateFMul(AddChain[15], AddChain[15]);
+  AddChain[31] = B.CreateFMul(AddChain[3],  AddChain[28]);
+  AddChain[32] = B.CreateFMul(AddChain[16], AddChain[16]);
+  return AddChain[Exp];
+}
----------------
majnemer wrote:
> You will end up creating unnecessary instructions if `Exp < 32`, please do not do this.
Thanks David.

My previous implementation used Binary Exponentiation but that results in more fmuls getting generated.

Addition-Chain Exponentiation generates the optimal (least) no. of fmuls.

Other reviewers were of the opinion that we should minimize fmuls (by precomputing the multiplication chains).

I think it will always be a trade-off between runtime and code-size; and here we opted for optimal runtime.


================
Comment at: test/Transforms/InstCombine/pow-4.ll:3
@@ +2,3 @@
+
+; RUN: opt -instcombine -S < %s | FileCheck %s --check-prefix=CHECK
+
----------------
majnemer wrote:
> You don't need --check-prefix=CHECK
I will fix this.


http://reviews.llvm.org/D13994