[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

pengfei added a comment. Thanks @clementval for reporting it and the reproducer. Put a patch D129294 to address it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107082/new/ https://reviews.llvm.org/D107082

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

clementval added a comment. @pengfei We are also hitting the following assertion with this patch. Do you have any idea why? /llvm-project/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:4333: void {anonymous}::SelectionDAGLegalize::ConvertNodeToLibcall(llvm::SDNode*): Assertion

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

pengfei added a comment. Thanks for confirming it! I don't have much experience in compiler-rt. But I think the version of clang matters much to compiler-rt particular in ABI changing cases like this :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

jeanPerier added a comment. In D107082#3632301 , @pengfei wrote: > Hi @jeanPerier , yes, you are right. This patch changes the calling > conversion of fp16 from GPRs to XMMs. So you need to update the runtime. If > you are using compiler-rt, you could

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

pengfei added a comment. Hi @jeanPerier , yes, you are right. This patch changes the calling conversion of fp16 from GPRs to XMMs. So you need to update the runtime. If you are using compiler-rt, you could simply re-build it with trunk code, or at least after rGabeeae57

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

jeanPerier added a comment. Hi @pengfei, I am working on flang, and after this patch, we started to see some bugs in Fortran programs using REAL(2) (which is fp16 in flang). I am not an expert in LLVM codegen and the builtins, but I am wondering if there is not issue with how llvm codegen

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

pengfei added a comment. In D107082#3628120 , @sylvestre.ledru wrote: > @pengfei I am not convinced it is an issue on my side. I don't have anything > particular in this area and using a stage2 build system. > > Anyway, this patch fixes the issue on my

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

sylvestre.ledru added a comment. @pengfei I am not convinced it is an issue on my side. I don't have anything particular in this area and using a stage2 build system. Anyway, this patch fixes the issue on my side:

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

pengfei added a comment. FYI, `COMPILER_RT_HAS_FLOAT16` is set according to https://github.com/llvm/llvm-project/blob/main/compiler-rt/cmake/builtin-config-ix.cmake#L25-L31 and https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/builtins/CMakeLists.txt#L699 Repository: rG LLVM

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

pengfei added a comment. In D107082#3626632 , @sylvestre.ledru wrote: > Same as in https://reviews.llvm.org/D114099 > It breaks the build on ubuntu bionic, Hirsute, etc on amd64: > > >

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

sylvestre.ledru added a comment. Same as in https://reviews.llvm.org/D114099 It breaks the build on ubuntu bionic, Hirsute, etc on amd64: "/build/llvm-toolchain-snapshot-15~++20220702091600+23ee84f43201/build-llvm/./bin/clang" --target=x86_64-pc-linux-gnu -DVISIBILITY_HIDDEN

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

pengfei added a comment. I'll take care next time. Thanks @MaskRay ! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107082/new/ https://reviews.llvm.org/D107082 ___ cfe-commits mailing list

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

MaskRay added a comment. In addition, don't use `Reland "Reland "Reland "Reland ...` One `Reland` is sufficient. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107082/new/ https://reviews.llvm.org/D107082

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

MaskRay added a comment. Please include `Differential Revision: ` line for reland commits as well so that people know that this patch has a reland. https://github.com/llvm/llvm-project/issues/56204 is related to 655ba9c8a1d22075443711cc749f0b032e07adee

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

pengfei added inline comments. Comment at: llvm/test/CodeGen/X86/fpclamptosat_vec.ll:605 +; CHECK-NEXT:.cfi_def_cfa_offset 80 +; CHECK-NEXT:movss %xmm2, {{[-0-9]+}}(%r{{[sb]}}p) # 4-byte Spill +; CHECK-NEXT:movss %xmm1, {{[-0-9]+}}(%r{{[sb]}}p) # 4-byte Spill

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

pengfei added a comment. In D107082#3576355 , @mehdi_amini wrote: > This broke the bot here: > https://lab.llvm.org/buildbot/#/builders/61/builds/27616 > > The cmake invocation includes some GPU specific options that you can omit >

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

mehdi_amini added a comment. This broke the bot here: https://lab.llvm.org/buildbot/#/builders/61/builds/27616 The cmake invocation includes some GPU specific options that you can omit (`-DMLIR_ENABLE_CUDA_RUNNER=1` , `-DCMAKE_CUDA_COMPILER=/usr/local/cuda/bin/nvcc`,

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2d2da259c872: [X86][RFC] Enable `_Float16` type support on X86 following the psABI (authored by pengfei). Changed prior to commit:

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

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

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

pengfei added inline comments. Comment at: llvm/test/CodeGen/X86/fpclamptosat_vec.ll:605 +; CHECK-NEXT:.cfi_def_cfa_offset 80 +; CHECK-NEXT:movss %xmm2, {{[-0-9]+}}(%r{{[sb]}}p) # 4-byte Spill +; CHECK-NEXT:movss %xmm1, {{[-0-9]+}}(%r{{[sb]}}p) # 4-byte Spill

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

pengfei added inline comments. Comment at: llvm/test/Analysis/CostModel/X86/fptoi_sat.ll:852 +; SSE2-NEXT: Cost Model: Found an estimated cost of 3 for instruction: %f16u1 = call i1 @llvm.fptoui.sat.i1.f16(half undef) +; SSE2-NEXT: Cost Model: Found an estimated cost of 5 for

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

pengfei updated this revision to Diff 435583. pengfei marked an inline comment as done. pengfei added a comment. Address Yuanke's comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107082/new/ https://reviews.llvm.org/D107082 Files:

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

LuoYuanke added inline comments. Comment at: llvm/test/CodeGen/X86/fpclamptosat.ll:569 ; CHECK-NEXT:cvttss2si %xmm0, %rax ; CHECK-NEXT:ucomiss {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0 +; CHECK-NEXT:movabsq $-9223372036854775808, %rcx # imm = 0x8000

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

LuoYuanke added inline comments. Comment at: llvm/test/Analysis/CostModel/X86/fptoi_sat.ll:852 +; SSE2-NEXT: Cost Model: Found an estimated cost of 3 for instruction: %f16u1 = call i1 @llvm.fptoui.sat.i1.f16(half undef) +; SSE2-NEXT: Cost Model: Found an estimated cost of 5

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

pengfei updated this revision to Diff 435151. pengfei marked 3 inline comments as done. pengfei added a comment. Address Yuanke's comments. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107082/new/ https://reviews.llvm.org/D107082 Files:

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

pengfei added inline comments. Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:616 +setOperationAction(ISD::FROUNDEVEN, MVT::f16, Promote); +setOperationAction(ISD::FP_ROUND, MVT::f16, Expand); +setOperationAction(ISD::FP_EXTEND, MVT::f32, Expand);

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

LuoYuanke added inline comments. Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:616 +setOperationAction(ISD::FROUNDEVEN, MVT::f16, Promote); +setOperationAction(ISD::FP_ROUND, MVT::f16, Expand); +setOperationAction(ISD::FP_EXTEND, MVT::f32, Expand);

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

pengfei updated this revision to Diff 430349. pengfei added a comment. Replace __gnu_f2h_ieee/__gnu_h2f_ieee with __truncsfhf2/__extendhfsf2 to match with GCC. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107082/new/

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

pengfei updated this revision to Diff 430314. pengfei added a comment. Use 32-bit spill slot for half type. Others still on going. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107082/new/ https://reviews.llvm.org/D107082 Files:

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

pengfei updated this revision to Diff 430019. pengfei added a comment. Herald added subscribers: armkevincheng, eric-k256, javed.absar. Herald added a reviewer: sjarus. Rebased on the avx512fp16 implementation. Still WIP for optimizations and a fast RA issue. Repository: rG LLVM Github

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

pengfei added a comment. After the last refactor, I think this patch is mostly ready. This patch strips most of the ABI and _Float16 type related code from D105263 , which can be leaving with only AVX512-FP16 ISA enabling code. I think it should be more

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

pengfei added a comment. In D107082#2913881 , @craig.topper wrote: > I haven't had a chance to look at this patch in detail, but I wanted to ask > if you considered doing what ARM and RISCV do for this. They pass the f16 in > the lower bits on an f32

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

craig.topper added a comment. I haven't had a chance to look at this patch in detail, but I wanted to ask if you considered doing what ARM and RISCV do for this. They pass the f16 in the lower bits on an f32 by only changing the ABI handling code in the backend. The type legalizer takes care

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

pengfei added inline comments. Comment at: llvm/include/llvm/IR/RuntimeLibcalls.def:293-294 HANDLE_LIBCALL(FPEXT_F16_F64, "__extendhfdf2") HANDLE_LIBCALL(FPEXT_F16_F32, "__gnu_h2f_ieee") HANDLE_LIBCALL(FPROUND_F32_F16, "__gnu_f2h_ieee") HANDLE_LIBCALL(FPROUND_F64_F16,

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

pengfei added a comment. I sent out this patch mainly for PoC of the ABI changes, I'll fix the performance regressions in next phase. LLVM was using a different calling conversion on x86 when passing and returning half type. It conflicts with current X86 psABI. I have evaluated the risk