[PATCH] D41608: [InstCombine] Missed optimization in math expression: pow multiplications

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

This patch is trying to do too many things at once. Please split only the 1st transform (pow(X, Y) * X -> pow(X, Y+1)) into its own patch, and let's continue looking at that alone as a 1st step. You should consider (and include tests for) these variations:

1. Commuted operands for fmul.
2. Extra uses of the pow result.
3. Vector types.
4. Additional FMF.

We don't necessarily need tests for every permutation of those, but there needs to be more coverage than what we see here currently.



================
Comment at: test/Transforms/InstCombine/fmul-pow.ll:28-29
 
 define double @pow_ab_a_reassoc_commute(double %a, double %b)  {
 ; CHECK-LABEL: @pow_ab_a_reassoc_commute(
+; CHECK-NEXT:    [[TMP1:%.*]] = fadd reassoc double [[B:%.*]], -1.000000e+00
----------------
This is a misleading test name. This isn't the commuted version of the previous test - the fmul does not have a common operand with the pow().

I think you want something like this:

```
declare double @call_f64(double)
define double @pow_ab_a_reassoc(double %p, double %b) {
  %a = call double @call_f64(double %p)  ; thwart complexity-based canonicalization
  %pow = call double @llvm.pow.f64(double %a, double %b)
  %mul = fmul reassoc double %a, %pow
  ret double %mul
}
```


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

https://reviews.llvm.org/D41608