[PATCH] D127050: [Clang][FP16] Add 4 builtins for _Float16

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.


================
Comment at: clang/test/CodeGen/builtin_Float16.c:7
+void test_float16_builtins(void) {
+  volatile _Float16 res;
+
----------------
LuoYuanke wrote:
> pengfei wrote:
> > LuoYuanke wrote:
> > > Is _Float16 a legal type for target armv7a and aarch64?
> > Yes, see https://clang.llvm.org/docs/LanguageExtensions.html#half-precision-floating-point
> Maybe use `__fp16` because it is supported on every target. 
No, I intentionally use `_Float16` here because I want to make it correspond to the type define `x` in Builtins.def
I think we should always use `x` (represents `_Float16`) rather than `h` (represents `__fp16`) when defining FP16 builtins. It doesn't make sense to to add new buildins for storage type, because all arithmetic operations should be promoted to `float`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127050