[PATCH] D57188: Disable _Float16 for non ARM/SPIR Targets

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Regardless, I think it would be fine if you wanted to add a CUDA/HIP exception 
to this check in the short term.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57188



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57188: Disable _Float16 for non ARM/SPIR Targets

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D57188#1374890 , @yaxunl wrote:

> This change causes regressions for CUDA/HIP. As single-source language, 
> CUDA/HIP code contains both device and host code. It has separate compilation 
> for host and device.
>  In host compilation, device function is parsed but not emitted in IR. The 
> device function may have _Float16 argument, which is fine if device target 
> supports it. Host compilation
>  should not diagnose use of _Float16 in device functions. However, current 
> implementation diagnose any _Float16 usage in host compilation.


Can this be reasonably delayed using the existing diagnostic-delay mechanism, 
or is there a problem where the diagnostics aren't necessarily bound to a 
function definition?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57188



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57188: Disable _Float16 for non ARM/SPIR Targets

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

This change causes regressions for CUDA/HIP. As single-source language, 
CUDA/HIP code contains both device and host code. It has separate compilation 
for host and device.
In host compilation, device function is parsed but not emitted in IR. The 
device function may have _Float16 argument, which is fine if device target 
supports it. Host compilation
should not diagnose use of _Float16 in device functions. However, current 
implementation diagnose any _Float16 usage in host compilation.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57188



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57188: Disable _Float16 for non ARM/SPIR Targets

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

"it" is the grammatically correct word.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57188



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57188: Disable _Float16 for non ARM/SPIR Targets

[Steve Canon via Phabricator via cfe-commits]

scanon added a subscriber: ab.
scanon added a comment.

> do we want to support _Float16 anywhere else?

ARM is the only in-tree target with a defined ABI that I'm aware of.

> Do we need to lock down an ABI here for i386/x86_64 in advance of those gears 
> turning in the outer world?

We definitely want to push for one to be defined (and make sure that it makes 
sense), but I don't think we don't need to rush ahead of everyone, rather get 
to preliminary agreement. I think @ab was going to follow-up on that?


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

https://reviews.llvm.org/D57188



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57188: Disable _Float16 for non ARM/SPIR Targets

[Erich Keane via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL352221: Disable _Float16 for non ARM/SPIR Targets (authored 
by erichkeane, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57188?vs=183536=183563#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57188

Files:
  cfe/trunk/docs/LanguageExtensions.rst
  cfe/trunk/include/clang/Basic/TargetInfo.h
  cfe/trunk/lib/Basic/TargetInfo.cpp
  cfe/trunk/lib/Basic/Targets/AArch64.cpp
  cfe/trunk/lib/Basic/Targets/ARM.cpp
  cfe/trunk/lib/Basic/Targets/SPIR.h
  cfe/trunk/lib/Sema/SemaType.cpp
  cfe/trunk/test/AST/float16.cpp
  cfe/trunk/test/CodeGenCXX/float16-declarations.cpp
  cfe/trunk/test/CodeGenCXX/mangle-ms.cpp
  cfe/trunk/test/Lexer/half-literal.cpp
  cfe/trunk/test/Sema/Float16.c

Index: cfe/trunk/lib/Sema/SemaType.cpp
===
--- cfe/trunk/lib/Sema/SemaType.cpp
+++ cfe/trunk/lib/Sema/SemaType.cpp
@@ -1441,7 +1441,12 @@
 else
   Result = Context.Int128Ty;
 break;
-  case DeclSpec::TST_float16: Result = Context.Float16Ty; break;
+  case DeclSpec::TST_float16:
+if (!S.Context.getTargetInfo().hasFloat16Type())
+  S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported)
+<< "_Float16";
+Result = Context.Float16Ty;
+break;
   case DeclSpec::TST_half:Result = Context.HalfTy; break;
   case DeclSpec::TST_float:   Result = Context.FloatTy; break;
   case DeclSpec::TST_double:
Index: cfe/trunk/lib/Basic/TargetInfo.cpp
===
--- cfe/trunk/lib/Basic/TargetInfo.cpp
+++ cfe/trunk/lib/Basic/TargetInfo.cpp
@@ -34,6 +34,7 @@
   NoAsmVariants = false;
   HasLegalHalfType = false;
   HasFloat128 = false;
+  HasFloat16 = false;
   PointerWidth = PointerAlign = 32;
   BoolWidth = BoolAlign = 8;
   IntWidth = IntAlign = 32;
Index: cfe/trunk/lib/Basic/Targets/AArch64.cpp
===
--- cfe/trunk/lib/Basic/Targets/AArch64.cpp
+++ cfe/trunk/lib/Basic/Targets/AArch64.cpp
@@ -49,6 +49,7 @@
 
   // All AArch64 implementations support ARMv8 FP, which makes half a legal type.
   HasLegalHalfType = true;
+  HasFloat16 = true;
 
   LongWidth = LongAlign = PointerWidth = PointerAlign = 64;
   MaxVectorAlign = 128;
Index: cfe/trunk/lib/Basic/Targets/ARM.cpp
===
--- cfe/trunk/lib/Basic/Targets/ARM.cpp
+++ cfe/trunk/lib/Basic/Targets/ARM.cpp
@@ -396,6 +396,7 @@
   SoftFloat = SoftFloatABI = false;
   HWDiv = 0;
   DotProd = 0;
+  HasFloat16 = true;
 
   // This does not diagnose illegal cases like having both
   // "+vfpv2" and "+vfpv3" or having "+neon" and "+fp-only-sp".
Index: cfe/trunk/lib/Basic/Targets/SPIR.h
===
--- cfe/trunk/lib/Basic/Targets/SPIR.h
+++ cfe/trunk/lib/Basic/Targets/SPIR.h
@@ -47,6 +47,7 @@
 AddrSpaceMap = 
 UseAddrSpaceMapMangling = true;
 HasLegalHalfType = true;
+HasFloat16 = true;
 // Define available target features
 // These must be defined in sorted order!
 NoAsmVariants = true;
Index: cfe/trunk/docs/LanguageExtensions.rst
===
--- cfe/trunk/docs/LanguageExtensions.rst
+++ cfe/trunk/docs/LanguageExtensions.rst
@@ -474,44 +474,58 @@
 =
 
 Clang supports two half-precision (16-bit) floating point types: ``__fp16`` and
-``_Float16``. ``__fp16`` is defined in the ARM C Language Extensions (`ACLE
-`_)
-and ``_Float16`` in ISO/IEC TS 18661-3:2015.
-
-``__fp16`` is a storage and interchange format only. This means that values of
-``__fp16`` promote to (at least) float when used in arithmetic operations.
-There are two ``__fp16`` formats. Clang supports the IEEE 754-2008 format and
-not the ARM alternative format.
-
-ISO/IEC TS 18661-3:2015 defines C support for additional floating point types.
-``_FloatN`` is defined as a binary floating type, where the N suffix denotes
-the number of bits and is 16, 32, 64, or greater and equal to 128 and a
-multiple of 32. Clang supports ``_Float16``. The difference from ``__fp16`` is
-that arithmetic on ``_Float16`` is performed in half-precision, thus it is not
-a storage-only format. ``_Float16`` is available as a source language type in
-both C and C++ mode.
-
-It is recommended that portable code use the ``_Float16`` type because
-``__fp16`` is an ARM C-Language Extension (ACLE), whereas ``_Float16`` is
-defined by the C standards committee, so using ``_Float16`` will not prevent
-code from being ported to architectures other than Arm.  

[PATCH] D57188: Disable _Float16 for non ARM/SPIR Targets

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: docs/LanguageExtensions.rst:484
+- SPIR
+``_Float16`` will be supported on more targets as they define ABIs for them.
+

rjmccall wrote:
> "them" should be "it" here.
Sorry, apparently Phabricator was sitting on a comment I made yesterday; ignore 
it, please, and just go with s/them/it/.


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

https://reviews.llvm.org/D57188



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57188: Disable _Float16 for non ARM/SPIR Targets

[John McCall via Phabricator via cfe-commits]

rjmccall added a subscriber: scanon.
rjmccall added a comment.

This LGTM with one minor revision; feel free to commit with that.

For follow-up commit consideration: @scanon, do we want to support `_Float16` 
anywhere else?  Do we need to lock down an ABI here for i386/x86_64 in advance 
of those gears turning in the outer world?




Comment at: docs/LanguageExtensions.rst:497
 defined by the C standards committee, so using ``_Float16`` will not prevent
-code from being ported to architectures other than Arm.  Also, ``_Float16``
-arithmetic and operations will directly map on half-precision instructions when
-they are available (e.g. Armv8.2-A), avoiding conversions to/from
-single-precision, and thus will result in more performant code. If
-half-precision instructions are unavailable, values will be promoted to
+code from being ported to architectures other than Arm, though ``_Float16`` is
+currently disabled on most platforms until support is added to their published

rjmccall wrote:
> This entire documentation section really just needs to be reworked.  Here's a 
> starting point:
> 
>   Clang supports two half-precision (16-bit) floating point types: ``__fp16`` 
> and
>   ``_Float16``.  These types are supported in all language modes.
> 
>   ``__fp16`` is supported on every target, as it is purely a storage format; 
> see below.
>   ``_Float16`` is currently only supported on the following targets:
>   - 32-bit ARM
>   - 64-bit ARM (AArch64)
>   - SPIR
>   ``_Float16`` will be supported on more targets as they define ABIs for them.
> 
>   ``__fp16`` is a storage and interchange format only.  This means that 
> values of
>   ``__fp16`` are immediately promoted to (at least) ``float`` when used in 
> arithmetic
>   operations, so that e.g. the result of adding two ``__fp16`` values has 
> type ``float``.
>   The behavior of ``__fp16`` is specified by the ARM C Language Extensions 
> (`ACLE 
> `_).
>   Clang uses the ``binary16`` format from IEEE 754-2008 for ``__fp16``, not 
> the ARM
>   alternative format.
> 
>   ``_Float16`` is an extended floating-point type.  This means that, just 
> like arithmetic on
>   ``float`` or ``double``, arithmetic on ``_Float16`` operands is formally 
> performed in the
>   ``_Float16`` type, so that e.g. the result of adding two ``_Float16`` 
> values has type
>   ``_Float16``.  The behavior of ``_Float16`` is specified by ISO/IEC TS 
> 18661-3:2015
>   ("Floating-point extensions for C").  As with ``__fp16``, Clang uses the 
> ``binary16``
>   format from IEEE 754-2008 for ``_Float16``.
> 
>   ``_Float16`` arithmetic will be performed using native half-precision 
> support
>   when available on the target (e.g. on ARMv8.2a); otherwise it will be 
> performed
>   at a higher precision (currently always ``float``) and then truncated down 
> to
>   ``_Float16``.  Note that C and C++ allow intermediate floating-point 
> operands
>   of an expression to be computed with greater precision than is expressible 
> in
>   their type, so Clang may avoid intermediate truncations in certain cases; 
> this may
>   lead to results that are inconsistent with native arithmetic.
> 
>   It is recommended that portable code use ``_Float16`` instead of ``__fp16``,
>   as it has been defined by the C standards committee and has behavior that is
>   more familiar to most programmers.
> 
>   Because ``__fp16`` operands are always immediately promoted to ``float``, 
> the
>   common real type of ``__fp16`` and ``_Float16`` for the purposes of the 
> usual
>   arithmetic conversions is ``float``.
> 
>   A literal can be given ``_Float16`` type using the suffix ``f16``; for 
> example:
>   ```
> 3.14f16
>   ```
> 
>   Because default argument promotion only applies to the standard 
> floating-point
>   types, ``_Float16`` values are not promoted to ``double`` when passed as 
> variadic
>   or untyped arguments.  As a consequence, some caution must be taken when 
> using
>   certain library facilities with ``_Float16``; for example, there is no 
> ``printf`` format
>   specifier for ``_Float16``, and (unlike ``float``) it will not be 
> implicitly promoted to
>   ``double`` when passed to ``printf``, so the programmer must explicitly 
> cast it to
>   ``double`` before using it with an ``%f`` or similar specifier.
Hmm.  Here's a better way of expressing part of that:

  ``_Float16`` is currently only supported on the following targets, with 
further
  targets pending ABI standardization:
  - 32-bit ARM
  - 64-bit ARM (AArch64)
  - SPIR



Comment at: docs/LanguageExtensions.rst:484
+- SPIR
+``_Float16`` will be supported on more targets as they define ABIs for them.
+

"them" should be "it" here.


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

https://reviews.llvm.org/D57188



___

[PATCH] D57188: Disable _Float16 for non ARM/SPIR Targets

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 183536.

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

https://reviews.llvm.org/D57188

Files:
  docs/LanguageExtensions.rst
  include/clang/Basic/TargetInfo.h
  lib/Basic/TargetInfo.cpp
  lib/Basic/Targets/AArch64.cpp
  lib/Basic/Targets/ARM.cpp
  lib/Basic/Targets/SPIR.h
  lib/Sema/SemaType.cpp
  test/AST/float16.cpp
  test/CodeGenCXX/float16-declarations.cpp
  test/CodeGenCXX/mangle-ms.cpp
  test/Lexer/half-literal.cpp
  test/Sema/Float16.c

Index: test/Sema/Float16.c
===
--- /dev/null
+++ test/Sema/Float16.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s -DHAVE
+// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s -DHAVE
+// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s -DHAVE
+
+#ifdef HAVE
+// expected-no-diagnostics
+#else
+// expected-error@+2{{_Float16 is not supported on this target}}
+#endif // HAVE
+_Float16 f;
Index: test/Lexer/half-literal.cpp
===
--- test/Lexer/half-literal.cpp
+++ test/Lexer/half-literal.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -pedantic %s
+// RUN: %clang_cc1 -fsyntax-only -verify -pedantic -triple aarch64-linux-gnu %s
 float a = 1.0h; // expected-error{{no matching literal operator for call to 'operator""h' with argument of type 'long double' or 'const char *', and no matching literal operator template}}
 float b = 1.0H; // expected-error{{invalid suffix 'H' on floating constant}}
 
Index: test/CodeGenCXX/mangle-ms.cpp
===
--- test/CodeGenCXX/mangle-ms.cpp
+++ test/CodeGenCXX/mangle-ms.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -fblocks -emit-llvm %s -o - -triple=i386-pc-win32 -std=c++98 | FileCheck %s
 // RUN: %clang_cc1 -fblocks -emit-llvm %s -o - -triple=x86_64-pc-win32 -std=c++98| FileCheck -check-prefix X64 %s
+// RUN: %clang_cc1 -fblocks -emit-llvm %s -o - -triple=aarch64-pc-win32 -std=c++98 -DARM | FileCheck -check-prefixes=X64,ARM %s
 
 int a;
 // CHECK-DAG: @"?a@@3HA"
@@ -466,10 +467,12 @@
 // CHECK-DAG: define dso_local void @"?f@Complex@@YAXU?$_Complex@H@__clang@@@Z"(
 void f(_Complex int) {}
 }
+#ifdef ARM
 namespace Float16 {
-// CHECK-DAG: define dso_local void @"?f@Float16@@YAXU_Float16@__clang@@@Z"(
+// ARM-DAG: define dso_local void @"?f@Float16@@YAXU_Float16@__clang@@@Z"(
 void f(_Float16) {}
 }
+#endif // ARM
 
 namespace PR26029 {
 template 
Index: test/CodeGenCXX/float16-declarations.cpp
===
--- test/CodeGenCXX/float16-declarations.cpp
+++ test/CodeGenCXX/float16-declarations.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang -std=c++11 --target=aarch64-arm--eabi -S -emit-llvm %s -o - | FileCheck %s  --check-prefix=CHECK --check-prefix=CHECK-AARCH64
-// RUN: %clang -std=c++11 --target=x86_64 -S -emit-llvm %s -o - | FileCheck %s  --check-prefix=CHECK --check-prefix=CHECK-X86
 
 /*  Various contexts where type _Float16 can appear. */
 
@@ -15,7 +14,6 @@
 
   _Float16 arr1n[10];
 // CHECK-AARCH64-DAG: @_ZN12_GLOBAL__N_15arr1nE = internal global [10 x half] zeroinitializer, align 2
-// CHECK-X86-DAG: @_ZN12_GLOBAL__N_15arr1nE = internal global [10 x half] zeroinitializer, align 16
 
   _Float16 arr2n[] = { 1.2, 3.0, 3.e4 };
 // CHECK-DAG: @_ZN12_GLOBAL__N_15arr2nE = internal global [3 x half] [half 0xH3CCD, half 0xH4200, half 0xH7753], align 2
@@ -30,14 +28,12 @@
 
 _Float16 f1f;
 // CHECK-AARCH64-DAG: @f1f = dso_local global half 0xH, align 2
-// CHECK-X86-DAG: @f1f = dso_local global half 0xH, align 2
 
 _Float16 f2f = 32.4;
 // CHECK-DAG: @f2f = dso_local global half 0xH500D, align 2
 
 _Float16 arr1f[10];
 // CHECK-AARCH64-DAG: @arr1f = dso_local global [10 x half] zeroinitializer, align 2
-// CHECK-X86-DAG: @arr1f = dso_local global [10 x half] zeroinitializer, align 16
 
 _Float16 arr2f[] = { -1.2, -3.0, -3.e4 };
 // CHECK-DAG: @arr2f = dso_local global [3 x half] [half 0xHBCCD, half 0xHC200, half 0xHF753], align 2
@@ -137,8 +133,6 @@
   long double cvtld = f2n;
 //CHECK-AARCh64-DAG: [[H2LD:%[a-z0-9]+]] = fpext half {{%[0-9]+}} to fp128
 //CHECK-AARCh64-DAG: store fp128 [[H2LD]], fp128* %{{.*}}, align 16
-//CHECK-X86-DAG: [[H2LD:%[a-z0-9]+]] = fpext half {{%[0-9]+}} to x86_fp80
-//CHECK-X86-DAG: store x86_fp80 [[H2LD]], x86_fp80* %{{.*}}, align 16
 
   _Float16 f2h = 42.0f;
 //CHECK-DAG: store half 0xH5140, half* %{{.*}}, align 2
Index: test/AST/float16.cpp
===
--- test/AST/float16.cpp
+++ test/AST/float16.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -std=c++11 -ast-dump %s | FileCheck %s --strict-whitespace
-// RUN: %clang_cc1 -std=c++11 -ast-dump -fnative-half-type %s | FileCheck %s 

[PATCH] D57188: Disable _Float16 for non ARM/SPIR Targets

[Erich Keane via Phabricator via cfe-commits]

erichkeane marked 5 inline comments as done.
erichkeane added inline comments.



Comment at: include/clang/Basic/TargetInfo.h:66
   bool HasFloat128;
+  bool HasFloat16;
   unsigned char PointerWidth, PointerAlign;

SjoerdMeijer wrote:
> I think this is the same as `HasLegalHalfType`, and we can (re)use that.  Or, 
> at least, don't think we need both `HasLegalHalfType` and `HasFloat16`. For 
> context, I needed `HasLegalHalfType` for argument passing, but it looks like 
> it can serve another purpose now.
> 
> Out of curiousity, I was wondering if specifying:
> 
>   KEYWORD(_Float16, HALFSUPPORT)
> 
> in TokenKids.def is an alternative approach (it is currently set to KEYALL). 
> Thus, enable the keyword when `LangOpts.Half` is set. By adding this 
> `HasFloat16` property here in clang's targetinfo, we're sort of defining 
> again how targets support different types. I.e., if you throw a `half` type 
> at the backend, the TypeLegalizer will deal with it in one way or another. 
> Perhaps disabling `_Float16` can be achieved by disabling the keyword. But I 
> do see that the big advantage of this patch is the much nicer error message 
> (otherwise we would get something like  "unknown type name '_Float16'").
> 
LangOpts.Half (and thus HALFSUPPORT) seems to be specific to OpenCL.  It seems 
to me that the two ARE different, the Half type and the _Float16 type are 
different semantically and from different standards, so I think that isn't 
sufficient.  

Additionally, from a diagnostics perspective I think it is worse to do it in 
the TokenKinds.def. If you do it in TokenKinds.def, you get "Type Expected" or 
"Unknown Type Name", which could encourage someone to write their own.  The 
error message in this patch (Not supported on this target) strongly states that 
it is a reserved word and is not to be defined.  This is particularly important 
for the targets that intend to add support for this in the future.

Additionally, HasLegalHalfType has slightly different semantics in the ARM 
case.  ARM seems to still want _Float16 to work even without the +fullfp16 
support.  It seems to be OK (based on CodeGen and the tests written to validate 
this) with allowing the _Float16 support to work with different code generation.


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

https://reviews.llvm.org/D57188



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57188: Disable _Float16 for non ARM/SPIR Targets

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.



Comment at: include/clang/Basic/TargetInfo.h:66
   bool HasFloat128;
+  bool HasFloat16;
   unsigned char PointerWidth, PointerAlign;

I think this is the same as `HasLegalHalfType`, and we can (re)use that.  Or, 
at least, don't think we need both `HasLegalHalfType` and `HasFloat16`. For 
context, I needed `HasLegalHalfType` for argument passing, but it looks like it 
can serve another purpose now.

Out of curiousity, I was wondering if specifying:

  KEYWORD(_Float16, HALFSUPPORT)

in TokenKids.def is an alternative approach (it is currently set to KEYALL). 
Thus, enable the keyword when `LangOpts.Half` is set. By adding this 
`HasFloat16` property here in clang's targetinfo, we're sort of defining again 
how targets support different types. I.e., if you throw a `half` type at the 
backend, the TypeLegalizer will deal with it in one way or another. Perhaps 
disabling `_Float16` can be achieved by disabling the keyword. But I do see 
that the big advantage of this patch is the much nicer error message (otherwise 
we would get something like  "unknown type name '_Float16'").




Comment at: include/clang/Basic/TargetInfo.h:521
+  /// Determine whether the _Float16 type is supported on this target.
+  virtual bool hasFloat16Type() const { return HasFloat16; }
+

Similar remark: the same as `hasLegalHalfType()`?


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

https://reviews.llvm.org/D57188



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57188: Disable _Float16 for non ARM/SPIR Targets

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: docs/LanguageExtensions.rst:497
 defined by the C standards committee, so using ``_Float16`` will not prevent
-code from being ported to architectures other than Arm.  Also, ``_Float16``
-arithmetic and operations will directly map on half-precision instructions when
-they are available (e.g. Armv8.2-A), avoiding conversions to/from
-single-precision, and thus will result in more performant code. If
-half-precision instructions are unavailable, values will be promoted to
+code from being ported to architectures other than Arm, though ``_Float16`` is
+currently disabled on most platforms until support is added to their published

This entire documentation section really just needs to be reworked.  Here's a 
starting point:

  Clang supports two half-precision (16-bit) floating point types: ``__fp16`` 
and
  ``_Float16``.  These types are supported in all language modes.

  ``__fp16`` is supported on every target, as it is purely a storage format; 
see below.
  ``_Float16`` is currently only supported on the following targets:
  - 32-bit ARM
  - 64-bit ARM (AArch64)
  - SPIR
  ``_Float16`` will be supported on more targets as they define ABIs for them.

  ``__fp16`` is a storage and interchange format only.  This means that values 
of
  ``__fp16`` are immediately promoted to (at least) ``float`` when used in 
arithmetic
  operations, so that e.g. the result of adding two ``__fp16`` values has type 
``float``.
  The behavior of ``__fp16`` is specified by the ARM C Language Extensions 
(`ACLE 
`_).
  Clang uses the ``binary16`` format from IEEE 754-2008 for ``__fp16``, not the 
ARM
  alternative format.

  ``_Float16`` is an extended floating-point type.  This means that, just like 
arithmetic on
  ``float`` or ``double``, arithmetic on ``_Float16`` operands is formally 
performed in the
  ``_Float16`` type, so that e.g. the result of adding two ``_Float16`` values 
has type
  ``_Float16``.  The behavior of ``_Float16`` is specified by ISO/IEC TS 
18661-3:2015
  ("Floating-point extensions for C").  As with ``__fp16``, Clang uses the 
``binary16``
  format from IEEE 754-2008 for ``_Float16``.

  ``_Float16`` arithmetic will be performed using native half-precision support
  when available on the target (e.g. on ARMv8.2a); otherwise it will be 
performed
  at a higher precision (currently always ``float``) and then truncated down to
  ``_Float16``.  Note that C and C++ allow intermediate floating-point operands
  of an expression to be computed with greater precision than is expressible in
  their type, so Clang may avoid intermediate truncations in certain cases; 
this may
  lead to results that are inconsistent with native arithmetic.

  It is recommended that portable code use ``_Float16`` instead of ``__fp16``,
  as it has been defined by the C standards committee and has behavior that is
  more familiar to most programmers.

  Because ``__fp16`` operands are always immediately promoted to ``float``, the
  common real type of ``__fp16`` and ``_Float16`` for the purposes of the usual
  arithmetic conversions is ``float``.

  A literal can be given ``_Float16`` type using the suffix ``f16``; for 
example:
  ```
3.14f16
  ```

  Because default argument promotion only applies to the standard floating-point
  types, ``_Float16`` values are not promoted to ``double`` when passed as 
variadic
  or untyped arguments.  As a consequence, some caution must be taken when using
  certain library facilities with ``_Float16``; for example, there is no 
``printf`` format
  specifier for ``_Float16``, and (unlike ``float``) it will not be implicitly 
promoted to
  ``double`` when passed to ``printf``, so the programmer must explicitly cast 
it to
  ``double`` before using it with an ``%f`` or similar specifier.



Comment at: lib/Basic/Targets/AArch64.cpp:52
   HasLegalHalfType = true;
+  HasFloat16= true;
 

spacing



Comment at: lib/Basic/Targets/SPIR.h:50
 HasLegalHalfType = true;
+HasFloat16= true;
 // Define available target features

spacing


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

https://reviews.llvm.org/D57188



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57188: Disable _Float16 for non ARM/SPIR Targets

[Erich Keane via Phabricator via cfe-commits]

erichkeane created this revision.
erichkeane added reviewers: rjmccall, andrew.w.kaylor.
Herald added subscribers: kristof.beyls, javed.absar.

As Discussed here:
http://lists.llvm.org/pipermail/llvm-dev/2019-January/129543.html

There are problems exposing the _Float16 type on architectures that
haven't defined the ABI/ISel for the type yet, so we're temporarily
disabling the type and making it opt-in.


https://reviews.llvm.org/D57188

Files:
  docs/LanguageExtensions.rst
  include/clang/Basic/TargetInfo.h
  lib/Basic/TargetInfo.cpp
  lib/Basic/Targets/AArch64.cpp
  lib/Basic/Targets/ARM.cpp
  lib/Basic/Targets/SPIR.h
  lib/Sema/SemaType.cpp
  test/AST/float16.cpp
  test/CodeGenCXX/float16-declarations.cpp
  test/CodeGenCXX/mangle-ms.cpp
  test/Lexer/half-literal.cpp
  test/Sema/Float16.c

Index: test/Sema/Float16.c
===
--- /dev/null
+++ test/Sema/Float16.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s -DHAVE
+// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s -DHAVE
+// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s -DHAVE
+
+#ifdef HAVE
+// expected-no-diagnostics
+#else
+// expected-error@+2{{_Float16 is not supported on this target}}
+#endif // HAVE
+_Float16 f;
Index: test/Lexer/half-literal.cpp
===
--- test/Lexer/half-literal.cpp
+++ test/Lexer/half-literal.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -pedantic %s
+// RUN: %clang_cc1 -fsyntax-only -verify -pedantic -triple aarch64-linux-gnu %s
 float a = 1.0h; // expected-error{{no matching literal operator for call to 'operator""h' with argument of type 'long double' or 'const char *', and no matching literal operator template}}
 float b = 1.0H; // expected-error{{invalid suffix 'H' on floating constant}}
 
Index: test/CodeGenCXX/mangle-ms.cpp
===
--- test/CodeGenCXX/mangle-ms.cpp
+++ test/CodeGenCXX/mangle-ms.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -fblocks -emit-llvm %s -o - -triple=i386-pc-win32 -std=c++98 | FileCheck %s
 // RUN: %clang_cc1 -fblocks -emit-llvm %s -o - -triple=x86_64-pc-win32 -std=c++98| FileCheck -check-prefix X64 %s
+// RUN: %clang_cc1 -fblocks -emit-llvm %s -o - -triple=aarch64-pc-win32 -std=c++98 -DARM | FileCheck -check-prefixes=X64,ARM %s
 
 int a;
 // CHECK-DAG: @"?a@@3HA"
@@ -466,10 +467,12 @@
 // CHECK-DAG: define dso_local void @"?f@Complex@@YAXU?$_Complex@H@__clang@@@Z"(
 void f(_Complex int) {}
 }
+#ifdef ARM
 namespace Float16 {
-// CHECK-DAG: define dso_local void @"?f@Float16@@YAXU_Float16@__clang@@@Z"(
+// ARM-DAG: define dso_local void @"?f@Float16@@YAXU_Float16@__clang@@@Z"(
 void f(_Float16) {}
 }
+#endif // ARM
 
 namespace PR26029 {
 template 
Index: test/CodeGenCXX/float16-declarations.cpp
===
--- test/CodeGenCXX/float16-declarations.cpp
+++ test/CodeGenCXX/float16-declarations.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang -std=c++11 --target=aarch64-arm--eabi -S -emit-llvm %s -o - | FileCheck %s  --check-prefix=CHECK --check-prefix=CHECK-AARCH64
-// RUN: %clang -std=c++11 --target=x86_64 -S -emit-llvm %s -o - | FileCheck %s  --check-prefix=CHECK --check-prefix=CHECK-X86
 
 /*  Various contexts where type _Float16 can appear. */
 
@@ -15,7 +14,6 @@
 
   _Float16 arr1n[10];
 // CHECK-AARCH64-DAG: @_ZN12_GLOBAL__N_15arr1nE = internal global [10 x half] zeroinitializer, align 2
-// CHECK-X86-DAG: @_ZN12_GLOBAL__N_15arr1nE = internal global [10 x half] zeroinitializer, align 16
 
   _Float16 arr2n[] = { 1.2, 3.0, 3.e4 };
 // CHECK-DAG: @_ZN12_GLOBAL__N_15arr2nE = internal global [3 x half] [half 0xH3CCD, half 0xH4200, half 0xH7753], align 2
@@ -30,14 +28,12 @@
 
 _Float16 f1f;
 // CHECK-AARCH64-DAG: @f1f = dso_local global half 0xH, align 2
-// CHECK-X86-DAG: @f1f = dso_local global half 0xH, align 2
 
 _Float16 f2f = 32.4;
 // CHECK-DAG: @f2f = dso_local global half 0xH500D, align 2
 
 _Float16 arr1f[10];
 // CHECK-AARCH64-DAG: @arr1f = dso_local global [10 x half] zeroinitializer, align 2
-// CHECK-X86-DAG: @arr1f = dso_local global [10 x half] zeroinitializer, align 16
 
 _Float16 arr2f[] = { -1.2, -3.0, -3.e4 };
 // CHECK-DAG: @arr2f = dso_local global [3 x half] [half 0xHBCCD, half 0xHC200, half 0xHF753], align 2
@@ -137,8 +133,6 @@
   long double cvtld = f2n;
 //CHECK-AARCh64-DAG: [[H2LD:%[a-z0-9]+]] = fpext half {{%[0-9]+}} to fp128
 //CHECK-AARCh64-DAG: store fp128 [[H2LD]], fp128* %{{.*}}, align 16
-//CHECK-X86-DAG: [[H2LD:%[a-z0-9]+]] = fpext half {{%[0-9]+}} to x86_fp80
-//CHECK-X86-DAG: store x86_fp80 [[H2LD]], x86_fp80* %{{.*}}, align 16
 
   _Float16 f2h = 42.0f;
 //CHECK-DAG: store half 0xH5140, half* %{{.*}}, align 2
Index: test/AST/float16.cpp