Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Topi][Relay] Support for FP16 ERF on CPU. #11413

Merged
merged 4 commits into from
May 23, 2022

Conversation

jwfromm
Copy link
Contributor

@jwfromm jwfromm commented May 23, 2022

X86 targets don't have an FP16 erf intrinsic. This causes compilation to fail when such an op is encountered. This PR adds two cases to avoid this failure. I've added an FP16 ERF legalization pass that will just cast to FP32 for the ERF call. I've also extended fast_erf to support FP16 directly. This does lose some precision compared to doing FP32 wrapping around ERF, but since the desire is to be faster I think it's acceptable behavior.

@jwfromm
Copy link
Contributor Author

jwfromm commented May 23, 2022

@anwang2009 can you take a look at this PR?

@anwang2009
Copy link
Contributor

Is the default behavior that FP16 ERF will fallback to FP32 cast for the ERF call? Basically wondering in what cases the different fallbacks are used?

@jwfromm
Copy link
Contributor Author

jwfromm commented May 23, 2022

There's different handling for relay.erf and relay.fast_erf. When an FP16 relay.erf is encountered, itll legalize it to cast(fp32) -> relay.erf -> cast(fp16). When an fp16 relay.fast_erf is encountered it will actually implement it in fp16.

Copy link
Contributor

@anwang2009 anwang2009 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotcha, LGTM!

@jwfromm jwfromm removed the request for review from AndrewZhaoLuo May 23, 2022 20:31
@AndrewZhaoLuo AndrewZhaoLuo merged commit 51c44ff into apache:main May 23, 2022
@jwfromm jwfromm deleted the fp16_fast_erf branch May 24, 2022 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants