-
Notifications
You must be signed in to change notification settings - Fork 108
Add f16
/f128
support
#1574
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
base: master
Are you sure you want to change the base?
Add f16
/f128
support
#1574
Conversation
LGTM for the non cranelift-specific parts, LLVM also does promote/demote for f16 powi and fmod. |
Is Cranelift ABI compatible with LLVM for f16 and f128 across all targets Cranelift supports? |
By architecture/platform:
Summary: All Cranelift |
Update: basic load/store/bitcast support is now available for all 4 architectures. Combined with the |
As a note, once the above lowering to rustc_codegen_cranelift/src/lib.rs Lines 200 to 208 in dfe7636
|
cfb192a
to
0e43247
Compare
fa5566b
to
bbb3d17
Compare
This PR now implements full support for |
I've discovered a pre-existing bug: when |
Would you mind splitting this into multiple commits? That would make it easier for me to review this. For example lib_call_arg_param can be a commit, adding fconst and basic float operations could be another one and implementing all the float intrinsics could be a third commit. And finally actually enabling support in CodegenBackend, the build system and the test code could be a final commit. |
Done. I've split it into 7 commits, but let me know if that's too many and I can squash some back together. |
} else { | ||
(value, AbiParam::new(types::F16)) | ||
}; | ||
fx.lib_call("__extendhfsf2", vec![arg_ty], vec![AbiParam::new(types::F32)], &[value])[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add these all new libcalls to the builtin_functions!
invocation in compiler_builtins.rs
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Not all currently used libcalls seem to be in there (e.g. fmod
, __powisf2
), so I've only added ones that seemed similar to the ones already there. Is this correct, or should every possible libcall be listed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The missing ones are just an oversight.
This PR adds full support for
f16
andf128
. As Cranelift backends only fully support basic operations such as load, store and bitcast at the moment, operations are polyfilled to allow full support for everything currently implemented inrustc
.Fixes #1461