[PATCH] D76077: [ARM] Add __bf16 as new Bfloat16 C Type

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8111
   "pointer cannot be cast to type %0">;
+def err_cast_to_bfloat : Error<"cannot type-cast to __bf16">;
+def err_cast_from_bfloat : Error<"cannot type-cast from __bf16">;
----------------
Nit: was wondering if `err_cast_to_bfloat16` would be more consistent


================
Comment at: clang/lib/AST/ASTContext.cpp:6007
     case Float16Rank:
     case HalfRank: llvm_unreachable("Complex half is not supported");
     case FloatRank:      return FloatComplexTy;
----------------
nit: perhaps this error message is not entirely accurate for bfloat16?


================
Comment at: clang/lib/Basic/Targets/AArch64.cpp:74
+  BFloat16Width = BFloat16Align = 16;
+  BFloat16Format = &llvm::APFloat::BFloat();
+
----------------
Nit: we use bfloat16 everywhere, would `llvm::APFloat::BFloat16()` be better for consistency?


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:115
   HalfTy = llvm::Type::getHalfTy(LLVMContext);
+  BFloatTy = llvm::Type::getBFloatTy(LLVMContext);
   FloatTy = llvm::Type::getFloatTy(LLVMContext);
----------------
nit: perhaps `getBFloat16Ty()`


================
Comment at: clang/lib/CodeGen/CodeGenTypeCache.h:39
+  /// half, bfloat, float, double
+  llvm::Type *HalfTy, *BFloatTy, *FloatTy, *DoubleTy;
 
----------------
And so here too, BFloat16Ty?


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:5844
   ABIKind Kind;
+  bool IsSoftFloatABI;
 
----------------
Nit: to distinguish the unfortunate float ABI names, I was wondering whether `IsFloatABISoftFP` is clearer, or something along those lines, just to make clear it is not the "soft" but the "softfp" variant


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76077