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

[Bryan Chan via Phabricator via llvm-commits]

bryanpkc added inline comments.


================
Comment at: compiler-rt/lib/builtins/fp_extend.h:43
 #elif defined SRC_HALF
+#if defined __aarch64__ && defined __ARM_FP16_ARGS
+// Use __fp16 if available.
----------------
SjoerdMeijer wrote:
> efriedma wrote:
> > Can we implement this some other way?  _Float16 should have the right calling convention on all targets, I think.
> That was exactly the first thing I was thinking of.  But then I also got confused about e.g. changing this:
> 
>   float __aeabi_h2f(uint16_t a)
> 
> to:  
> 
>   float __aeabi_h2f(src a)
> 
> where src can now be __fp16. First I was wondering if changing the signature would be problematic somehow. Probably not, and probably it  is more correct. Looking at the RT ABI for h2f, the descriptions reads:
> 
>     IEEE 754 binary16 storage format (VFP half precision) to binary32 (float)  conversion
> 
> From a first glance, I could image that __fp16 would capture this behaviour, but I would need to check again what the binary16 storage format exactly is, and that's why I wasn't sure about all this.
> 
Thanks for your comments. Many of these FP16 conversion helpers should be unnecessary on AArch64 (when `__ARM_FP16_ARGS` is defined) since the backend can generate instructions for such conversions. However the implementations of the helpers are intended to be shared by all targets, so that is why I decided to limit the impact of my change with the conditional compilation directive above. Similarly I also concluded that changing the signature of `__aeabi_h2f` etc. would not be problematic for AArch64 with FP16 support, but to be conservative, we could change `src_t` back to `uint16_t` in the standard runtime helpers, and add an explicit cast of `a` from `uint16_t` to `src_t` for the call to `__extendhfsf2` etc.


Repository:
  rL LLVM

https://reviews.llvm.org/D50685