[PATCH] D41342: [InstCombine] Missed optimization in math expression: simplify calls exp functions

Wed Jan 30 05:52:41 PST 2019

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

LGTM, but please wait for @spatel.

================
Comment at: test/Transforms/InstCombine/fmul-exp.ll:62
+;
+  %tmp = call fast double @llvm.exp.f64(double %a)
+  %tmp1 = call fast double @llvm.exp.f64(double %b)
----------------
spatel wrote:
> Quolyk wrote:
> > spatel wrote:
> > > We don't need the full "fast" to enable these transforms. Can you adjust the tests to show the minimal amount of FMF necessary to allow the optimizations? We need "reassoc" + anything else?
> > > 
> > > Do you have commit access? If so, please commit these files to trunk with the current (without this code patch) output.
> > I have commit access, but I don't want to commit without patch approval. Revision is fixed according to your comments.
> Thanks. I think the patch is close to approval, so you should commit the tests now. There are 2 benefits to committing the tests first:
> 1. You and the reviewers can see the actual diffs that will occur with the code change. Ie, if something in a test is not already in canonical form, you can correct or show that in the baseline test commit.
> 2. If the code change must be reverted for some reason, we should not lose the regression test coverage because of that revert.
> We need "reassoc" + anything else?

Any further thoughts here? Just "reassoc" is enough?
(that is what is currently being done)

Repository:
  rL LLVM

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

https://reviews.llvm.org/D41342