[PATCH] D86453: [AArch64] Support conversion between fp16 and fp128

Tue Sep 8 11:52:00 PDT 2020

zatrazz marked an inline comment as done.
zatrazz added inline comments.

================
Comment at: compiler-rt/lib/builtins/fp_extend.h:43
 #elif defined SRC_HALF
+#if defined COMPILER_RT_HAS_FLOAT16
+typedef _Float16 src_t;
----------------
MaskRay wrote:
> MaskRay wrote:
> > `#ifdef` ? ditto below
> You can mark the comment above as "Done".
> 
> _Float16 and uint16_t are different types. Do all the `*hf*` functions change their signatures when `_Float16` is supported?  This still looks strange.
I think using _Float16 for generic support does make more sense, since float16 support is really an extension and with different calling convention on each architecture and even on some different ABIs. While it is uint16_t for armv6 due the soft-fp ABI, it is a complete different type with a different calling convention for aarch64. And I expect that other architectures to use similar strategies now that it is supported on some vector extensions in some new chips (POWER10 for instance).

Also, for ABI which expects *hf* to have 'uint16_t' as calling convetion it would be better to compartmentalize it to the specific arch folder, for instance for arm (and this is what libgcc does for instance).

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86453/new/

https://reviews.llvm.org/D86453