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.

Sign up for GitHub

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

Jump to bottom

Switch Float16 LLVM representation from i16 to half #26381

Closed
wants to merge 7 commits into from
Closed

Switch Float16 LLVM representation from i16 to half #26381

wants to merge 7 commits into from

Conversation

vchuravy
Copy link
Member

@vchuravy vchuravy commented Mar 9, 2018 •
edited
Loading

This is a first step towards implementing the parts of #18927 and #18734
in the laziest most straight-forward possible.

  1. Use LLVM's half type for Float16
  2. Use LLVM intrinsics for basic operations on Float16
  3. Provide fallbacks for methods missing in our current libc.

This foregoes the completnes aspect of #18927, while providing
a way forward to potentially extend this to Float128, and it doesn't add
additional dependencies like #18734.

Follow-up PR's could extend RTLIB to be more generic or to not use Base
(thus allowing it to be it's own shared library). There are other places
where Float16 currently are eagerly converted to Float32 so this is
not an attempt at completness.

All the code paths are tested on x86 Linux and I am currently testing on ARM.
(There is a known PPC codegen issue, but I guess nobody but me cares about that).

@vtjnash is there anything I would need to do for anticodegen/LLVM free builds?

Notes:

vchuravy added 4 commits March 8, 2018 19:11
@vchuravy
[RTLIB] start implementing soft float16 support.
ae7cad9 
Some LLVM intrinsics can't be lowered to instructions on target
platforms. Some of these are therefore lowered to libc libcalls
and glibc does not implement all of them. This commit uses `extern_c`
to implement these fallback methods in native Julia.

Similar projects to RTLIB are glibc and compiler-rt.

For now we only implement conversion between `Float16` and `Float32`,
we also cheat by going through `Float32` for the conversion between
`Float64` and `Float16`.
@vchuravy
resolve libcalls to sys.so
6a556f3 
LLVM intrinsics either map to instructions or to functions in
compiler-rt. Since we provide our own implementation we can just look
them up in sys.so and resolve to the function there.

On Darwin we have to use a unmangled version of the function name.
@vchuravy
switch Float16 from i16 to half
eebad3e
@vchuravy
switch Float16 to LLVM intrinsics
8a9abe7
This was referenced Mar 9, 2018
Closed
Closed
vchuravy
vchuravy commented Mar 9, 2018
View reviewed changes
base/rtlib/RTLIB.jl Outdated

# We would like to use `@ccallable` here,
# but building the sysimage fails, so we use a bootstrapped version.
function register(f, rtype, argt, name)
Copy link
Member Author

Choose a reason for hiding this comment

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

@vtjnash I was trying to use @ccallable here but I was getting failures during sysimage building.

@vchuravy vchuravy requested a review from vtjnash March 9, 2018 00:33
@vchuravy
Copy link
Member Author

vchuravy commented Mar 9, 2018

@vtjnash On AArch64 I am running into:

Wrong types for attribute: byval inalloca nest noalias nocapture nonnull readnone readonly signext sret zeroext dereferenceable(1) dereferenceable_or_null(1)
float (half)* @jlcapi_extendhfsf2_3064
LLVM ERROR: Broken function found, compilation aborted!

@vchuravy
Copy link
Member Author

vchuravy commented Mar 9, 2018

Further update on PPC situation. The backend can't select FP16 operations (even on master), so I will first need to work with upstream on adding support for that.
That means that this PR is currently blocked until I get around to that :(, except if somebody else has a clever idea.

I wouldn't want to simply disable this on PPC since that would inhibit GPU code on PPC from using Float16.

@vchuravy
Copy link
Member Author

vchuravy commented Mar 9, 2018

Today does not seem to be my day. It only worked beautifully on my tests because I have the F16C instruction set extension.
After fighting with recursive definitions I am now stuck on a seqfault that is probably related to my usage of jl_extern_c.

@stevengj
Copy link
Member

stevengj commented Mar 9, 2018

The title of this PR refers to switching the representation. Does this actually change the Float16 format, or does it only change the algorithms for working with Float16?

@yuyichao
Copy link
Contributor

yuyichao commented Mar 9, 2018

I believe it changes the representation in LLVM but it shouldn't change the bit pattern.

@vchuravy vchuravy changed the title Switch Float16 representation to half Switch Float16 LLVM representation from i16 to half Mar 9, 2018
@vchuravy vchuravy mentioned this pull request Mar 13, 2018
Open
17 tasks
@philtomson
Copy link

What's the status of this PR? (conflicts, of course, but is anyone working on it?)

@vchuravy
Copy link
Member Author

vchuravy commented Sep 17, 2018 •
edited
Loading

Still blocked today on #26381 (comment), but I would welcome another set of eyes to look at this, I would like to see this happening, but I can't dedicate time to it.

@jekbradbury
Copy link
Contributor

Is there any way to disable it on PPC host but allow it to stay on for the NVPTX backend used by LLVM.jl for GPU code?

@vchuravy
Copy link
Member Author

No I don't think that is feasible we have CodegenParameters but that would require a fair amount of refactoring to get it to all places it needs to be, and it is unclear how we would handle it in pure Julia... It might be feasible to use i16 on PPC for host and GPU (e.g. turn float16 off completely).

@KristofferC KristofferC mentioned this pull request Nov 1, 2018
Closed
@KristofferC KristofferC mentioned this pull request Jan 8, 2019
Closed
@ViralBShah
Copy link
Member

Close this?

@vchuravy
Copy link
Member Author

vchuravy commented May 5, 2019 via email

@vtjnash vtjnash removed their request for review August 5, 2019 13:39
vtjnash
vtjnash requested changes Aug 5, 2019
View reviewed changes
Copy link
Member

@vtjnash vtjnash left a comment •
edited by ViralBShah
Loading

Choose a reason for hiding this comment

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

@vchuravy says this PR is outdated

@maleadt maleadt mentioned this pull request Nov 13, 2019
Merged
@vchuravy vchuravy mentioned this pull request Nov 28, 2019
Merged
@maleadt maleadt mentioned this pull request Aug 25, 2020
Open
@maleadt maleadt mentioned this pull request Sep 10, 2020
Merged
@vchuravy
Copy link
Member Author

Closed in favour of #37510

@vchuravy vchuravy closed this Sep 10, 2020
@DilumAluthge DilumAluthge deleted the vc/float16 branch March 25, 2021 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vtjnash vtjnash vtjnash requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@vchuravy @stevengj @yuyichao @philtomson @jekbradbury @ViralBShah @vtjnash