[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