Page MenuHomePhabricator

[InstCombine] Missed optimization in math expression: simplify calls exp functions
Needs ReviewPublic

Authored by Quolyk on Dec 18 2017, 1:21 AM.

Details

Summary

This patch enables folding following expressions under -ffast-math flag: exp(X) * exp(Y) -> exp(X + Y), exp2(X) * exp2(Y) -> exp2(X + Y)

Diff Detail

Event Timeline

Quolyk created this revision.Dec 18 2017, 1:21 AM
hfinkel added inline comments.Dec 19 2017, 8:03 PM
lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
457

Needs

BuilderTy::FastMathFlagGuard Guard(Builder);

first (so that the flags will get unset in the builder as required).

459

Line is too long.

hfinkel added inline comments.Dec 19 2017, 8:13 PM
lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
456

Also, ->hasOneUse() checks here.

Quolyk updated this revision to Diff 127665.Dec 20 2017, 12:55 AM
Quolyk retitled this revision from [WIP][InstCombine] Missed optimization in math expression: simplify calls exp functions to [InstCombine] Missed optimization in math expression: simplify calls exp functions.

Since I'm new to llvm, I looked though other test files, and noticed that it isn't necessary to write full test-check, instead people write just one instruction to make sure test is ok, while consequent instructions doesn't matter. I wonder what style is preferred?

Quolyk marked 3 inline comments as done.Dec 20 2017, 12:56 AM

Since I'm new to llvm, I looked though other test files, and noticed that it isn't necessary to write full test-check, instead people write just one instruction to make sure test is ok, while consequent instructions doesn't matter. I wonder what style is preferred?

Checks used to be mostly written by hand (and had a lot of shortcomings and bugs because of that). Now that we have scripts to auto-generate checks, I prefer that you use those scripts when possible. It should make subsequent changes or additions to the test file easier for other contributors.

Quolyk updated this revision to Diff 127832.Dec 20 2017, 11:45 PM
Quolyk updated this revision to Diff 128408.Jan 2 2018, 4:22 AM
Quolyk edited the summary of this revision. (Show Details)
Quolyk updated this revision to Diff 129399.Jan 11 2018, 12:00 AM
Quolyk updated this revision to Diff 135633.Feb 23 2018, 6:22 AM
Quolyk edited reviewers, added: efriedma; removed: davide.
Quolyk updated this revision to Diff 155848.Jul 17 2018, 4:13 AM
lebedev.ri added inline comments.
test/Transforms/InstCombine/fmul-exp.ll
15

Please cleanup testcases, name all the variables (-instnamer will help)
(When committing, please commit tests first, so the code change shows the test change)

30–31

I think this can still be transformed, as long as at least one of them has only one use.
This would be transformed into

%2 = call fast double @llvm.exp.f64(double %b)
call void @use(double %2)
%t1 = add fast double %a, %b
%t2 = call fast double @llvm.exp.f64(double %t2)
ret double %t2

which has the same number of instruction,
And add a test where both of them are two-use, which won't be transformed.

test/Transforms/InstCombine/fmul-exp2.ll
68

Please add newlines