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

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

There are Sema and CodeGen tests, but was wondering if there would be some value in having an AST test too? There are some other types that do have AST tests.



================
Comment at: clang/lib/AST/ASTContext.cpp:2052
+      Width = Target->getBFloat16Width();
+      Align = Target->getBFloat16Align();
     case BuiltinType::Float16:
----------------
Is a `break` missing here?


================
Comment at: clang/lib/CodeGen/CodeGenTypes.cpp:307
+    else
+      return llvm::Type::getInt16Ty(VMContext);
+  }
----------------
Is this covered in tests?


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:6454
+    // types into integer vectors.
+    // We do not depend on haslegalhalf type for bfloat as it is a
+    // separate IR type
----------------
nit: camelcase fase for haslegalhalf


================
Comment at: clang/test/CodeGen/arm-mangle-16bit-float.cpp:4
+
+// CHECK64: define {{.*}}void @_Z3foou6__bf16(half %b)
+// CHECK32: define {{.*}}void @_Z3foou6__bf16(i32 %b.coerce)
----------------
LukeGeeson wrote:
> craig.topper wrote:
> > How can bfloat16 be passed as half? Don't they have a different format?
> see the above comment about soft-abis
Probably I am bit confused too now... are the three possible types that we are expecing not bfloat, i32, or i16? 


================
Comment at: clang/test/CodeGen/arm-mangle-16bit-float.cpp:2
+// RUN: %clang_cc1 -triple aarch64-arm-none-eabi -fallow-half-arguments-and-returns -target-feature +bf16 -target-feature +fullfp16 -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK64
+// RUN: %clang_cc1 -triple arm-arm-none-eabi     -fallow-half-arguments-and-returns -target-feature +bf16 -target-feature +fullfp16 -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK32
+
----------------
Do you need to pass `+fullfp16`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76077