[PATCH] D44561: [ARM] Add HasFloat16 to TargetInfo

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.


================
Comment at: include/clang/Basic/TargetInfo.h:365
 
+  /// \brief Determine whether _Float16 is supported on this target.
+  virtual bool hasFloat16Type() const { return HasFloat16; }
----------------
`_Float16` doesn't seem to be supported anywhere in Clang (`__fp16` is).

But we should probably clarify exactly what kind of support we mean here. This variable doesn't affect:

  * Whether __fp16 can be used at all in source.
  * Its ABI.

I'm actually slightly worried that when we document what it does affect it'll end up being an ARM implementation-detail.




================
Comment at: lib/Basic/Targets/ARM.cpp:382
   HWDiv = 0;
-  HasFullFP16 = 0;
+  HasFloat16 = 0;
 
----------------
This is now a bool rather than a bitfield, so `false` is definitely the best initializer (probably was before, too, but whatever).


================
Comment at: lib/CodeGen/CGBuiltin.cpp:3456
   case NeonTypeFlags::Float16:
-    // FIXME: Only AArch64 backend can so far properly handle half types.
-    // Remove else part once ARM backend support for half is complete.
-    if (Arch == llvm::Triple::aarch64)
+    if (Arch == llvm::Triple::aarch64 || Arch == llvm::Triple::aarch64_be ||
+        HasFloat16)
----------------
This check is equivalent to setting `HasFloat16` in `AArch64TargetInfo` at the moment. Are you intending to change that (say by adding more uses of `HasFloat16`)?


https://reviews.llvm.org/D44561