This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Add support for lowering tanh to LLVMIR.
ClosedPublic

Authored by hanchung on Jun 10 2020, 3:57 PM.

Details

Summary

Add a pattern for expanding tanh op into exp form.
A tanh is expanded into:

  1. 1-exp^{-2x} / 1+exp^{-2x}, if x => 0
  2. exp^{2x}-1 / exp^{2x}+1 , if x < 0.

Diff Detail

Event Timeline

hanchung created this revision.Jun 10 2020, 3:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 10 2020, 3:57 PM

Didn't go through the code yet. high level comments:

  • Is it possible to get this lowered to a tanh function call instead of the exp rewrite ?
  • If we have to rewrite tanh(x) then lets express it in a numerically stable way because tanh(x) = exp^{2x}-1 / exp^{2x}+1 will blow up for relatively not very big positive values of x, so we can do:
tanh(x) = 1-exp^{-2x} / 1+exp^{-2x}, x => 0
tanh(x) = exp^{2x}-1 / exp^{2x}+1, x < 0
ftynse requested changes to this revision.Jun 11 2020, 1:17 AM

It looks like this is better done as an intra-dialect transform, everything that you need is available in the standard dialect so the code can just expand std.tanh to a set of std. operations and then use the regular conversion. We already do so for atomics https://github.com/llvm/llvm-project/blob/master/mlir/lib/Dialect/StandardOps/Transforms/ExpandAtomic.cpp, we can put ExpandTanh.cpp next to it.

This revision now requires changes to proceed.Jun 11 2020, 1:17 AM

Is it possible to get this lowered to a tanh function call instead of the exp rewrite ?

Unless we assume libc is linked, there's no tanh function.

Is it possible to get this lowered to a tanh function call instead of the exp rewrite ?

Unless we assume libc is linked, there's no tanh function.

other functions e.g llvm.inter.exp aren't libc mathlib calls ?

Is it possible to get this lowered to a tanh function call instead of the exp rewrite ?

Unless we assume libc is linked, there's no tanh function.

other functions e.g llvm.inter.exp aren't libc mathlib calls ?

It depends on the LLVM backend. LLVM models _all_ intrinsics as function calls, but it does not necessarily mean they actually become function calls in the generated assembly (e.g., llvm.intr.fma won't).

hanchung updated this revision to Diff 270278.Jun 11 2020, 6:43 PM

Add expanding tanh pattern to StandardOps/Transforms/

hanchung updated this revision to Diff 270280.Jun 11 2020, 6:45 PM

Add a test

hanchung updated this revision to Diff 270284.Jun 11 2020, 6:56 PM

Add missing files...

Thanks for the advice, I think it's better now. :)

ftynse requested changes to this revision.Jun 12 2020, 2:16 AM

Thanks!

mlir/include/mlir/Dialect/StandardOps/Transforms/Passes.h
30

Does something actually use it as a pass? I only see the pattern to be used inside Linalg conversion. If there is no direct user of this pass, let's put it under test/lib/Transforms.

mlir/lib/Dialect/StandardOps/Transforms/ExpandTanh.cpp
2

It does not perform loop fusion ;)

36

Please don't commit commented-out code

44

Could we have a better name? Like doubledX ?

This revision now requires changes to proceed.Jun 12 2020, 2:16 AM
asaadaldien added inline comments.Jun 12 2020, 9:20 AM
mlir/lib/Dialect/StandardOps/Transforms/ExpandTanh.cpp
38

return failure() if operand isn't float ?

hanchung updated this revision to Diff 270480.Jun 12 2020, 12:06 PM
hanchung marked 10 inline comments as done.

Address comments.

mlir/include/mlir/Dialect/StandardOps/Transforms/Passes.h
30

Done, good to know this, thank you!

mlir/lib/Dialect/StandardOps/Transforms/ExpandTanh.cpp
2

Good catch, thanks! I copied it from ExpandAtomic, let me send out another patch to fix that part.

36

Forgot to remove debug code, done.

38

I think we don't need this check because it's guaranteed that the type of the operand must be float.

class FloatUnaryOp<string mnemonic, list<OpTrait> traits = []> :
    UnaryOpSameOperandAndResultType<mnemonic, traits>,
    Arguments<(ins FloatLike:$operand)>;
...
def TanhOp : FloatUnaryOp<"tanh">
 ...
44

Done, also fixed the name in the test.

hanchung retitled this revision from [mlir][StandardToLLVM] Add support for lowering tanh to LLVMIR. to [mlir] Add support for lowering tanh to LLVMIR..Jun 12 2020, 12:09 PM
hanchung edited the summary of this revision. (Show Details)
ftynse accepted this revision.Jun 13 2020, 2:44 AM

Thanks for iterating!

This revision is now accepted and ready to land.Jun 13 2020, 2:44 AM
This revision was automatically updated to reflect the committed changes.

Reverted in a9a21bb4b682474248dc85f9e7db4b260d249ab9 ; the standalone test was broken

Reverted in a9a21bb4b682474248dc85f9e7db4b260d249ab9 ; the standalone test was broken

It passed with bazel on my local machine, I'll take a look at what happened, sorry about that.