This is an archive of the discontinued LLVM Phabricator instance.

Add pattern that expands `math.roundeven` into `math.round` + arith
ClosedPublic

Authored by ramiro050 on Apr 13 2023, 6:26 PM.

Details

Summary

This commit adds a pattern that expands math.roundeven into
math.round + some ops from arith. This is needed to be able to run
math.roundeven in a vectorized manner.

Diff Detail

Event Timeline

ramiro050 created this revision.Apr 13 2023, 6:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 13 2023, 6:26 PM
ramiro050 requested review of this revision.Apr 13 2023, 6:26 PM
rsuderman added inline comments.Apr 14 2023, 7:10 AM
mlir/lib/Dialect/Math/Transforms/ExpandPatterns.cpp
246

Couldn't we just use math::CopysignOp?

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

You will want to CHECK-DAG most of this computation if you can. This way you can break the checks into blocks. I often find it handy with explaining the sections of IR and their high level goal.

mlir/test/mlir-cpu-runner/expand-math-ops.mlir
1

There should be a new file like this that has this now at head (was just landed yesterday). If you could merge yours and it, that would be fantastic.

Addressed comments

ramiro050 marked 2 inline comments as done.Apr 20 2023, 12:19 PM
jpienaar accepted this revision.Apr 20 2023, 12:28 PM

Nice, thanks!

This revision is now accepted and ready to land.Apr 20 2023, 12:28 PM
mehdi_amini added inline comments.Apr 20 2023, 11:17 PM
mlir/test/mlir-cpu-runner/test-expand-math-approx.mlir
378 ↗(On Diff #515440)

This last check here is failing on my Mac and prints a positive nan

ramiro050 added inline comments.Apr 21 2023, 8:36 AM
mlir/test/mlir-cpu-runner/test-expand-math-approx.mlir
378 ↗(On Diff #515440)

I don't really understand why that would happen. When the input is +-nan, the expand pattern for roundeven simplifies to an identity operation. For example, if I apply to the expand pattern to

func.func @main(%val : f32) -> f32 {
  %result = math.roundeven %val : f32
  func.return %result : f32
}

and then I define %val inside the function and canonicalize, I get

module {
  func.func @main() -> f32 {
    %cst = arith.constant 0xFFC00000 : f32
    return %cst : f32
  }
}

Moreover, the expand pattern explicitly copies the sign from the operand at the end before returning. Does math.copysign -nan -nan produce +nan on Mac?

ramiro050 added inline comments.Apr 21 2023, 9:49 AM
mlir/test/mlir-cpu-runner/test-expand-math-approx.mlir
378 ↗(On Diff #515440)

Actually, the IEEE 754-2008 standard does not require the negative sign when printing a positive or negative nan:

Conversion of a quiet NaN in a supported format to an external character sequence shall produce a language-defined one of “nan” or a sequence that is equivalent except for case (e.g., “NaN”), with an optional preceding sign. (This standard does not interpret the sign of a NaN.)

So I think the correct way of fixing this is to just change the CHECK to nan.