[PATCH] D113107: Support of expression granularity for _Float16.

zahiraam updated this revision to Diff 411177. zahiraam set the repository for this revision to rG LLVM Github Monorepo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113107/new/ https://reviews.llvm.org/D113107 Files: clang/docs/LanguageExtension

[PATCH] D113107: Support of expression granularity for _Float16.

zahiraam added a comment. @rjmccall @pengfei @andrew.w.kaylor review please? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113107/new/ https://reviews.llvm.org/D113107 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm

[PATCH] D113107: Support of expression granularity for _Float16.

zahiraam updated this revision to Diff 394224. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113107/new/ https://reviews.llvm.org/D113107 Files: clang/docs/LanguageExtensions.rst clang/docs/ReleaseNotes.rst clang/lib/Basic/Targets/X86.cpp clang/lib/Sema/SemaExpr.cpp clang/test/

[PATCH] D113107: Support of expression granularity for _Float16.

rjmccall added a comment. For that example, yes, approach #3 would result in that exact same IR on targets that lack direct hardware support for `_Float16` operations. But getting that behavior right in general requires a different implementation than is provided by this patch, which is implem

[PATCH] D113107: Support of expression granularity for _Float16.

pengfei added inline comments. Comment at: clang/test/CodeGen/X86/Float16-aritmetic.c:8-9 + // CHECK: alloca half + // CHECK: store half {{.*}}, half* + // CHECK: store half {{.*}}, half* + // CHECK: load half, half* zahiraam wrote: > pengfei wrote: > > This

[PATCH] D113107: Support of expression granularity for _Float16.

pengfei added a comment. > Should also generate this same IR right? For `a + b + c`, yes, they are the same. For `a + b`, no. That's the difference between approaches #3 and #4, which I didn't emphasize before :) https://godbolt.org/z/onbGeqdYv CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D113107: Support of expression granularity for _Float16.

zahiraam added a comment. Question about this comment: 3. “ Approaches #1 and #2 require a lot of intermediate conversions when hardware isn't available. In our example, a + b + c has to be calculated as (_Float16) ((float) (_Float16) ((float) a + (float) b) + (float) c), where the result of o

[PATCH] D113107: Support of expression granularity for _Float16.

pengfei added a comment. Thanks for @rjmccall 's summary and @scanon 's information. They are exactly the backgrounds I know for the requirment. Sorry I didn't and couldn't say it clearly in this way. @zahiraam , I took `__fp16` for example to illustrate the case for `a + b + c`, but didn't emp

[PATCH] D113107: Support of expression granularity for _Float16.

scanon added a comment. In D113107#3138671 , @rjmccall wrote: >> Basically agree with everything John said, with a note that #3 is not quite >> FP_CONTRACT, which allows evaluating an expression as if intermediate steps >> were infinitely-precise, but r

[PATCH] D113107: Support of expression granularity for _Float16.

rjmccall added a comment. > Basically agree with everything John said, with a note that #3 is not quite > FP_CONTRACT, which allows evaluating an expression as if intermediate steps > were infinitely-precise, but rather FLT_EVAL_METHOD == 32 as defined in > ISO/IEC TS 18661-3: "evaluate operati

[PATCH] D113107: Support of expression granularity for _Float16.

zahiraam added a comment. In D113107#3138415 , @rjmccall wrote: > In D113107#3137921 , @zahiraam > wrote: > >> In D113107#3136464 , @rjmccall >> wrote: >> >>> Does GCC a

[PATCH] D113107: Support of expression granularity for _Float16.

scanon added a comment. In D113107#3138415 , @rjmccall wrote: > I think we keep dancing around this in this review, so let me go back and > start from the basics. There are four approaches I know of for evaluating a > homogeneous `_Float16` expression

[PATCH] D113107: Support of expression granularity for _Float16.

rjmccall added a comment. In D113107#3137921 , @zahiraam wrote: > In D113107#3136464 , @rjmccall > wrote: > >> Does GCC actually change the formal types of expressions to `float`, or is >> it doing this "no inte

[PATCH] D113107: Support of expression granularity for _Float16.

zahiraam added a comment. In D113107#3136464 , @rjmccall wrote: > Does GCC actually change the formal types of expressions to `float`, or is it > doing this "no intermediate casts thing" as some sort of fp_contract-style > custom emission of trees of ex

[PATCH] D113107: Support of expression granularity for _Float16.

zahiraam added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1315 + if ((SrcType->isHalfType() || iSFloat16Allowed) && + !CGF.getContext().getLangOpts().NativeHalfType) { // Cast to FP using the intrinsic if the half type itself isn't supported. -

[PATCH] D113107: Support of expression granularity for _Float16.

pengfei added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1315 + if ((SrcType->isHalfType() || iSFloat16Allowed) && + !CGF.getContext().getLangOpts().NativeHalfType) { // Cast to FP using the intrinsic if the half type itself isn't supported. --

[PATCH] D113107: Support of expression granularity for _Float16.

rjmccall added a comment. Does GCC actually change the formal types of expressions to `float`, or is it doing this "no intermediate casts thing" as some sort of fp_contract-style custom emission of trees of expressions that involve `_Float16`? In any case, doing operation-by-operation emulation

[PATCH] D113107: Support of expression granularity for _Float16.

zahiraam added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1315 + if ((SrcType->isHalfType() || iSFloat16Allowed) && + !CGF.getContext().getLangOpts().NativeHalfType) { // Cast to FP using the intrinsic if the half type itself isn't supported. -

[PATCH] D113107: Support of expression granularity for _Float16.

pengfei added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1315 + if ((SrcType->isHalfType() || iSFloat16Allowed) && + !CGF.getContext().getLangOpts().NativeHalfType) { // Cast to FP using the intrinsic if the half type itself isn't supported. --

[PATCH] D113107: Support of expression granularity for _Float16.

rjmccall added a reviewer: scanon. rjmccall added a comment. + Steve Canon Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1315 + if ((SrcType->isHalfType() || iSFloat16Allowed) && + !CGF.getContext().getLangOpts().NativeHalfType) { // Cast to FP using the intrinsic i

[PATCH] D113107: Support of expression granularity for _Float16.

pengfei added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1315 + if ((SrcType->isHalfType() || iSFloat16Allowed) && + !CGF.getContext().getLangOpts().NativeHalfType) { // Cast to FP using the intrinsic if the half type itself isn't supported. --

[PATCH] D113107: Support of expression granularity for _Float16.

rjmccall added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1315 + if ((SrcType->isHalfType() || iSFloat16Allowed) && + !CGF.getContext().getLangOpts().NativeHalfType) { // Cast to FP using the intrinsic if the half type itself isn't supported. -

[PATCH] D113107: Support of expression granularity for _Float16.

pengfei added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1313 + !CGF.getContext().getTargetInfo().hasFeature("avx512fp16") && + (TT.isX32() || TT.isX86()); + if ((SrcType->isHalfType() || iSFloat16Allowed) && We don't need to check

[PATCH] D113107: Support of expression granularity for _Float16.

rjmccall added a comment. Can we split the patch which changes what types are enabled for various x86 sub-targets out from the patch that changes the semantics of operations on `_Float16`? Or is there a good reason it's disabled currently, namely because the semantics are wrong? ===

[PATCH] D113107: Support of expression granularity for _Float16.

zahiraam added inline comments. Comment at: clang/test/CodeGen/X86/avx512fp16-complex.c:123 // X86-NOT: fdiv - // X86: call {{.*}} @__divhc3( + // X86: call {{.*}} @__divsc3( // X86: ret zahiraam wrote: > pengfei wrote: > > andrew.w.kaylor wrote: > > > Wh

[PATCH] D113107: Support of expression granularity for _Float16.

zahiraam updated this revision to Diff 387387. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113107/new/ https://reviews.llvm.org/D113107 Files: clang/lib/Basic/Targets/X86.cpp clang/lib/CodeGen/CGExprScalar.cpp clang/lib/Sema/SemaExpr.cpp clang/test/CodeGen/X86/Float16-aritmetic

<    1   2