This is an archive of the discontinued LLVM Phabricator instance.

[MLIR][Math] Add erf to math dialect
ClosedPublic

Authored by sogartar on Oct 20 2021, 10:32 PM.

Details

Summary

Add math.erf lowering to libm call.
Add math.erf polynomial approximation.

Diff Detail

Event Timeline

sogartar created this revision.Oct 20 2021, 10:32 PM
sogartar requested review of this revision.Oct 20 2021, 10:32 PM
silvas accepted this revision.Oct 21 2021, 11:25 AM

LGTM. Probably wait for another pair of eyes to LGTM it. @ezhulenev perhaps?

mlir/lib/Dialect/Math/Transforms/PolynomialApproximation.cpp
507

nit: can you mention the source of the approximation?

mlir/test/Analysis/test-shape-fn-report.mlir
15 ↗(On Diff #381147)

nit: I wouldn't add this test in this patch (it's redundant with the tanh one)

This revision is now accepted and ready to land.Oct 21 2021, 11:25 AM
ezhulenev accepted this revision.Oct 21 2021, 2:41 PM
ezhulenev added inline comments.
mlir/lib/Dialect/Math/Transforms/PolynomialApproximation.cpp
190

Maybe pass ArrayRef<Value> instead of a pair of iterators and a template parameter? Both call sites seems to pass values.

sogartar updated this revision to Diff 381444.Oct 21 2021, 5:46 PM

Remove math.erf test from test-shape-fn-report.mlir.
Update math.erf polynomial approximation description.
Use ArrayRef in makePolynomialCalculation.

sogartar marked 2 inline comments as done.Oct 21 2021, 5:48 PM
sogartar added inline comments.
mlir/lib/Dialect/Math/Transforms/PolynomialApproximation.cpp
507

I used Boost's minimax tool that utilizes the Remez method to find the constants. I will update the comment with the info.

sogartar marked 2 inline comments as done.Oct 21 2021, 5:49 PM
sogartar requested review of this revision.Oct 22 2021, 9:14 AM

I addressed the issues in the initial review.

I will expose the approximation of math.erf separately.

sogartar updated this revision to Diff 381683.Oct 22 2021, 4:38 PM

Expose math.erf polynomial approximation.

sogartar updated this revision to Diff 381684.Oct 22 2021, 4:49 PM

Add using namespace mlir::math to PolynomialApproximation.cpp.

This revision was not accepted when it landed; it landed in state Needs Review.Oct 25 2021, 11:30 AM
This revision was landed with ongoing or failed builds.
Closed by commit rGf1b922188ead: [MLIR][Math] Add erf to math dialect (authored by sogartar, committed by silvas). · Explain Why
This revision was automatically updated to reflect the committed changes.