[PATCH] D41608: [WIP][InstCombine] Missed optimization in math expression: aggressive optimization with pow

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

Why is this patch WIP? The mentioned case of 'pow(a, x) * a * a * a * a -> pow(a, x+4)' is handled already by this patch.



================
Comment at: lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:705-708
+    Value *A0 = nullptr;
+    Value *B0 = nullptr;
+    Value *A1 = nullptr;
+    Value *B1 = nullptr;
----------------
You mentioned that you see warnings without explicitly setting these to nullptr in Xcode. I use Xcode as an IDE too, but I don't see any warnings like that when I remove the nullptrs. 

If this is really a problem, then you must be getting hundreds of these warnings for existing code that does not initialize things like this? I'd prefer not to bloat the code unnecessarily.


================
Comment at: lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:713
+                                           Intrinsic::pow, I.getType());
+    // pow(A, B) * A -> pow(A, B+1)
+    if (match(Op0, m_Intrinsic<Intrinsic::pow>(m_Value(A0), m_Value(B0))) &&
----------------
Need to handle commuted versions too (please add a test):

```
define double @pow_ab_x_a_fast_commute(double %a, double %b)  {
  %c = fdiv double 1.0, %a  ; defeat complexity-based canonicalization of operands
  %p = call fast double @llvm.pow.f64(double %a, double %b)
  %mul = fmul fast double %c, %p
  ret double %mul
}

```


================
Comment at: lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:714-715
+    // pow(A, B) * A -> pow(A, B+1)
+    if (match(Op0, m_Intrinsic<Intrinsic::pow>(m_Value(A0), m_Value(B0))) &&
+        A0 == Op1) {
+      Value *One = ConstantFP::get(Op0->getType(), 1.0);
----------------
Here and below: use m_Specific instead of the trailing check for equality?


================
Comment at: lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:723-724
+    // pow(A, B) * pow(C, B) -> pow(A*C, B)
+    if (match(Op0, m_Intrinsic<Intrinsic::pow>(m_Value(A0), m_Value(B0))) &&
+        match(Op1, m_Intrinsic<Intrinsic::pow>(m_Value(A1), m_Value(B1))) &&
+        B0 == B1) {
----------------
Please use variable names that match the formulas in the code comments for better readability.


https://reviews.llvm.org/D41608