This is an archive of the discontinued LLVM Phabricator instance.

[mlir][math] Expand math.round to truncate, compare and increment.
ClosedPublic

Authored by bviyer on Apr 11 2023, 9:50 AM.

Details

Summary

Round functions are pushed directly to libm. This is problematic for
situations where libm is not available. This patch will decompose the
roundf function by adding 0.5 to positive number to input
(subtracting for negative) following by a truncate.

Diff Detail

Event Timeline

bviyer created this revision.Apr 11 2023, 9:50 AM
bviyer requested review of this revision.Apr 11 2023, 9:50 AM

Just the one fix and we should be good.

mlir/test/Dialect/Math/expand-math.mlir
177

Rather than CHECK-NEXT just use CHECK-DAG on everything except the return. That way we don't enforce strict ordering on operations when not necessary.

bviyer updated this revision to Diff 512940.Apr 12 2023, 12:23 PM
bviyer marked an inline comment as done.

Replaced all-but-last CHECK-NEXT with CHECK-DAG as requested by rsuderman

rsuderman added inline comments.Apr 12 2023, 1:48 PM
mlir/test/Dialect/Math/expand-math.mlir
182

This should be CHECK

mlir/test/mlir-cpu-runner/math-polynomial-approx.mlir
690 ↗(On Diff #512940)

So I realized a problem. These tests are in polynomial approximation but should be in a separate CPU runner test. Specifically one that validates expand-math separately works. Before landing these two CLs, could have split out the arith lowering tests to a separate file and substitute test-math-polynomial-approximation for test-expand-math and add the equivalent arith expansion.

rsuderman added inline comments.Apr 12 2023, 1:51 PM
mlir/test/mlir-cpu-runner/math-polynomial-approx.mlir
690 ↗(On Diff #512940)

So just to note, we will want both one for arith's expand patterns and one for math's expand patterns.

bviyer updated this revision to Diff 513304.Apr 13 2023, 10:45 AM
bviyer marked 3 inline comments as done.

Added the test to the new file and also replaced CHECK-NEXT with CHECK.

Just one small change.

mlir/test/mlir-cpu-runner/math-polynomial-approx.mlir
687–731 ↗(On Diff #513304)

We should remove this as it is in the expand math tests now.

bviyer updated this revision to Diff 513307.Apr 13 2023, 10:56 AM

Removed func_roundf and roundf

bviyer marked an inline comment as done.Apr 13 2023, 10:57 AM
rsuderman accepted this revision.Apr 13 2023, 10:58 AM
This revision is now accepted and ready to land.Apr 13 2023, 10:58 AM
This revision was landed with ongoing or failed builds.Apr 13 2023, 11:02 AM
This revision was automatically updated to reflect the committed changes.