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; }
----------------
t.p.northover wrote:
> SjoerdMeijer wrote:
> > t.p.northover wrote:
> > > `_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.
> > >
> > >
> > I've added _Float16 support in Clang commit r312794: "Add _Float16 as a
> > C/C++ source language type" :-)
> Oh wow, that was ages ago! I have no idea how I still had a repo that old
> hanging around. Sorry about that.
I'm afraid I still think this description is iffy. For a start it is actually
still affecting `__fp16` rather than `Float16` (since that's what float16_t is
in the NEON header). And it's still too vague about just what "support" implies.
================
Comment at: lib/CodeGen/CGBuiltin.cpp:3444
NeonTypeFlags TypeFlags,
llvm::Triple::ArchType Arch,
+ bool HasFloat16=true,
----------------
Now that you set `HasFloat16` in AArch64, I believe `Arch` is unused.
https://reviews.llvm.org/D44561
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
