rogfer01 added inline comments.
================
Comment at: include/clang/AST/Type.h:1669
bool isHalfType() const; // OpenCL 6.1.1.1, NEON (IEEE 754-2008 half)
+ bool isFloat16Type() const; // FIXME
bool isRealType() const; // C99 6.2.5p17 (real floating + integer)
----------------
I think you want to make clear that this is
```
// C11 extension ISO/IEC TS 18661-3
```
================
Comment at: lib/AST/StmtPrinter.cpp:1434
default: llvm_unreachable("Unexpected type for float literal!");
+ case BuiltinType::Float16:
case BuiltinType::Half: break; // FIXME: suffix?
----------------
Should this be `.f16` as suffix for consistency with the floating literal
syntax?
================
Comment at: lib/CodeGen/CGExprScalar.cpp:1932-1934
+ else if (value->getType()->isFloat16Ty()) {
+ FS = &CGF.getTarget().getHalfFormat(); //FIXME?
+ } else
----------------
I think you don't need this new LLVM type (that you introduce in D34205) as you
are already able to tell the two types apart (`_fp16` and `_Float16`) at the
level of clang types. And your change does not seem to need it in any other
place.
================
Comment at: lib/CodeGen/CodeGenTypes.cpp:445
+ getTypeForFormat(getLLVMContext(), Context.getFloatTypeSemantics(T),
+ true);
+ break;
----------------
I think you can make this more obvious to the reader with a comment for this
bool parameter.
```
/* UseNativeHalf */ true
```
https://reviews.llvm.org/D33719
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
