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

This revision was automatically updated to reflect the committed changes. Closed by commit rGecd682bbf5e6: [ARM] Add __bf16 as new Bfloat16 C Type (authored by stuij). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76077/new/

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

stuij updated this revision to Diff 268614. stuij marked an inline comment as done. stuij added a comment. Resolved merge conflicts with head. Fixed minor oversight. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76077/new/

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

stuij marked 2 inline comments as done. stuij added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:6989 + false, + getTarget().hasBFloat16Type()); llvm::Type *Ty = VTy;

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

labrinea added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:6989 + false, + getTarget().hasBFloat16Type()); llvm::Type *Ty = VTy; shouldn't this be

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

SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. Thanks, LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76077/new/ https://reviews.llvm.org/D76077

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

stuij updated this revision to Diff 268335. stuij added a comment. Addressed review comments. Notably what to do in combination with -mfloat-abi=soft. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76077/new/ https://reviews.llvm.org/D76077

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

stuij added a comment. In D76077#2066520 , @SjoerdMeijer wrote: > > At the moment when going through the GCC compatibility driver (standard > > interface), we get __bf16 is not supported on this target. > > If this is the current behaviour, and

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

SjoerdMeijer added a comment. > At the moment when going through the GCC compatibility driver (standard > interface), we get __bf16 is not supported on this target. If this is the current behaviour, and consistent with GCC, that sounds reasonable. Probably best to be explicit about this and

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

stuij marked an inline comment as done. stuij added a comment. In D76077#2066323 , @SjoerdMeijer wrote: > Can you summarise where we are? I.e., > > - float-abi=soft doesn't work. But what is the problem? Are we not simply > passing i16s, is that not

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

SjoerdMeijer added a comment. Can you summarise where we are? I.e., - float-abi=soft doesn't work. But what is the problem? Are we not simply passing i16s, is that not what we are supposed to do? Can you also update the description of this patch, I got totally confused by: - "introduces an

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

stuij marked 2 inline comments as done. stuij added inline comments. Comment at: clang/lib/Basic/Targets/ARM.cpp:80 DoubleAlign = LongLongAlign = LongDoubleAlign = SuitableAlign = 32; + BFloat16Width = BFloat16Align = 16; + BFloat16Format = ::APFloat::BFloat();

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

labrinea added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:4493 + case NeonTypeFlags::BFloat16: +if (HasBFloat16Type) + return llvm::VectorType::get(CGF->BFloatTy, V1Ty ? 1 : (4 << IsQuad)); This is not what we should be checking for

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

SjoerdMeijer added inline comments. Comment at: clang/test/CodeGen/arm-bf16-params-returns.c:5 +// RUN: %clang_cc1 -triple aarch64-arm-none-eabi -target-abi aapcs -mfloat-abi softfp -target-feature +bf16 -target-feature +neon -emit-llvm -O2 -o - %s | opt -S -mem2reg -sroa |

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

stuij marked 9 inline comments as done. stuij added inline comments. Comment at: clang/lib/Basic/Targets/AArch64.cpp:74 + BFloat16Width = BFloat16Align = 16; + BFloat16Format = ::APFloat::BFloat(); + SjoerdMeijer wrote: > stuij wrote: > > SjoerdMeijer wrote: >

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

SjoerdMeijer added inline comments. Comment at: clang/test/CodeGen/arm-mangle-bf16.cpp:3 +// RUN: %clang_cc1 -triple arm-arm-none-eabi -target-feature +bf16 -mfloat-abi hard -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK32-HARD +// RUN: %clang_cc1 -triple

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

SjoerdMeijer added a comment. As I wrote in one of the inlined comments, I am relatively unhappy that we have both bfloat and bfloat16 as names. But that looks like a complain for another patch, and not really this one. Comment at: clang/lib/Basic/Targets/AArch64.cpp:74 +

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

stuij 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">;

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

stuij updated this revision to Diff 265985. stuij marked 14 inline comments as done. stuij added a comment. addressed review comments and some related general changes: - renamed IsSoftFloatABI -> IsFloatABISoftFP - split bfloat tests out of arm-mangle-16bit-float.cpp - bfloat shouldn't be

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

stuij updated this revision to Diff 265753. stuij marked an inline comment as done. stuij added a comment. no explicit float-abi cmdline arg should default to softfp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76077/new/

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

LukeGeeson added inline comments. 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) SjoerdMeijer wrote: > LukeGeeson wrote: > >

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

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 =

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

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">;

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

SjoerdMeijer added inline comments. Comment at: clang/docs/LanguageExtensions.rst:518 +Clang supports three half-precision (16-bit) floating point types: ``__fp16``, +``_Float16`` and ``__bf16``. These types are supported in all language modes. stuij wrote: >

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

stuij marked an inline comment as done. stuij added inline comments. Comment at: clang/docs/LanguageExtensions.rst:518 +Clang supports three half-precision (16-bit) floating point types: ``__fp16``, +``_Float16`` and ``__bf16``. These types are supported in all language modes.

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

SjoerdMeijer added inline comments. Comment at: clang/docs/LanguageExtensions.rst:518 +Clang supports three half-precision (16-bit) floating point types: ``__fp16``, +``_Float16`` and ``__bf16``. These types are supported in all language modes. stuij wrote: >

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

stuij updated this revision to Diff 264560. stuij added a comment. addressing review comments and adding Simon Tatham to contributers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76077/new/ https://reviews.llvm.org/D76077 Files:

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

stuij updated this revision to Diff 264562. stuij added a comment. redo: addressing review comments and adding Simon Tatham to contributers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76077/new/ https://reviews.llvm.org/D76077 Files:

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

stuij marked 4 inline comments as done. stuij added a comment. @LukeGeeson: you're already mentioned :) See the commits tab on this review. But Simon Tatham needs a mention as well. I shall add him. Comment at: clang/docs/LanguageExtensions.rst:518 +Clang supports three

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

SjoerdMeijer added inline comments. Comment at: clang/docs/LanguageExtensions.rst:518 +Clang supports three half-precision (16-bit) floating point types: ``__fp16``, +``_Float16`` and ``__bf16``. These types are supported in all language modes. Not my field

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

majnemer added inline comments. Comment at: clang/include/clang-c/Index.h:3254 CXType_FirstBuiltin = CXType_Void, CXType_LastBuiltin = CXType_ULongAccum, Should this be: CXType_LastBuiltin = CXType_BFloat16, Comment at:

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

LukeGeeson added a comment. I authored some of the code for this patch, please update the commit message with my name on the list :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76077/new/ https://reviews.llvm.org/D76077

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

stuij updated this revision to Diff 263719. stuij added a comment. changes: - addressed asmith's comments - Bfloat doesn't depend on half cmdline options and correct code is generation for AArch32 hard and softfp CC Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

stuij marked 2 inline comments as done. stuij added a comment. @asmith: Thanks for reviewing :) Fixed your suggestions in the coming revision. Comment at: clang/include/clang/AST/ASTContext.h:965 CanQualType HalfTy; // [OpenCL 6.1.1.1], ARM NEON + CanQualType BFloat16Ty;

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

asmith added a comment. really happy for native bfloat support! Comment at: clang/include/clang/AST/ASTContext.h:965 CanQualType HalfTy; // [OpenCL 6.1.1.1], ARM NEON + CanQualType BFloat16Ty; // ARM NEON CanQualType Float16Ty; // C11 extension ISO/IEC TS 18661-3

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

stuij updated this revision to Diff 261851. stuij added a comment. I've amended this patch to conform to the general Bfloat -> BFloat convention changes requested in the dependent BFloat IR patch (D78190 ). I also folded in two other downstream patches, which

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

stuij updated this revision to Diff 258018. stuij added a comment. changes: depend on bfloat IR type, and resolve some conflicts that crept in Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76077/new/ https://reviews.llvm.org/D76077 Files:

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

stuij commandeered this revision. stuij edited reviewers, added: LukeGeeson; removed: stuij. stuij added a comment. commandeering as I've got some changes to push CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76077/new/ https://reviews.llvm.org/D76077

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

stuij added a comment. changes in the last patch: depend on bfloat IR type, and resolve some conflicts that crept in CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76077/new/ https://reviews.llvm.org/D76077 ___ cfe-commits mailing list

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

SjoerdMeijer added a comment. That sounds good and that seems to address the feedback given here, which I agree with. Just wanted to briefly add that while it already looks like most interested people are on this ticket, perhaps it good to also share the plan and design on the cfe dev list for

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

stuij added a comment. Thanks for the feedback. We'll work on implementing a bfloat IR type. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76077/new/ https://reviews.llvm.org/D76077 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

jfb added a comment. In D76077#1921973 , @rjmccall wrote: > In D76077#1921490 , @LukeGeeson > wrote: > > > In D76077#1919861 , @rjmccall > > wrote: > > > > > I don't

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

LukeGeeson updated this revision to Diff 250267. LukeGeeson added a comment. Added a file from a downstream cleanup of the branch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76077/new/ https://reviews.llvm.org/D76077 Files: clang/docs/LanguageExtensions.rst

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

rjmccall added a comment. In D76077#1921490 , @LukeGeeson wrote: > In D76077#1919861 , @rjmccall wrote: > > > I don't understand why you wouldn't add a new IR type for this; doing so > > should be totally

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

LukeGeeson added a comment. In D76077#1919861 , @rjmccall wrote: > I don't understand why you wouldn't add a new IR type for this; doing so > should be totally mechanical. We had a few reasons for doing it this way. By adding a new IR type we would

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

LukeGeeson marked an inline comment as done. LukeGeeson added inline comments. 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)

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

LukeGeeson added a comment. In D76077#1919897 , @rjmccall wrote: > Note that we have an IR type for the PPC double-double format, which isn't > even hardware-supported. This is literally just an IEEE floating-point > format with non-standard

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

rjmccall added a comment. Note that we have an IR type for the PPC double-double format, which isn't even hardware-supported. This is literally just an IEEE floating-point format with non-standard parameters, right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

rjmccall added a comment. I don't understand why you wouldn't add a new IR type for this; doing so should be totally mechanical. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76077/new/ https://reviews.llvm.org/D76077

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

craig.topper added inline comments. Comment at: clang/lib/Basic/Targets/AArch64.cpp:69 + Bfloat16Width = Bfloat16Align = 16; + Bfloat16Format = ::APFloat::IEEEhalf(); + Doesn't Bfloat16 have a different number of mantissa and exponent bits than IEEEhalf?

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

LukeGeeson created this revision. LukeGeeson added reviewers: SjoerdMeijer, stuij, rjmccall, rsmith, liutianle, RKSimon, craig.topper. Herald added subscribers: cfe-commits, danielkiss, arphaman, kristof.beyls. Herald added a project: clang. LukeGeeson added a parent revision: D76062: [PATCH]