[PATCH] D151284: [RISCV] Custom lower FP_TO_FP16 and FP16_TO_FP to correct ABI of of libcall

Alex Bradbury via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 23 20:55:16 PDT 2023

asb created this revision.
asb added reviewers: craig.topper, reames, kito-cheng.
Herald added subscribers: jobnoorman, luke, wingo, pmatos, VincentWu, armkevincheng, sjarus, eric-k256, vkmr, frasercrmck, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, niosHD, sabuasal, simoncook, johnrusso, rbar, hiraditya, arichardson.
Herald added a project: All.
asb requested review of this revision.
Herald added subscribers: pcwang-thead, eopXD, MaskRay.
Herald added a project: LLVM.

As introduced in D99148 <https://reviews.llvm.org/D99148>, RISC-V uses the softPromoteHalf legalisation for fp16 values without zfh, with logic ensuring that f16 values are passed in lower bits of FPRs (see D98670 <https://reviews.llvm.org/D98670>) when F or D support is present. This legalisation produces ISD::FP_TO_FP16 and ISD::FP16_TO_FP nodes which (as described in ISDOpcodes.h) provide a "semi-softened interface for dealing with f16 (as an i16)". i.e. the return type of the FP_TO_FP16 is an integer rather than a float (and the arg of FP16_TO_FP is an integer). The remainder of the description focuses primarily on FP_TO_FP16 for ease of explanation.

FP_TO_FP16 is lowered to a libcall to __truncsfhf2 (float) or __truncdfhf2 (double). As of D92241 <https://reviews.llvm.org/D92241>, `_Float16` is used as the return type of these libcalls if the host compiler accepts _Float16 in a test input (i.e. dst_t is set to _Float16). _Float16 is enabled for the RISC-V target as of D105001 <https://reviews.llvm.org/D105001> and so the return value should be passed in an FPR on hard float ABIs.

This patch fixes the ABI issue in what appears to be a minimally invasive way - leaving the softPromoteHalf logic undisturbed, and lowering FP_TO_FP16 to an f32-returning libcall, converting its result to an XLen integer value. Alternate strategies might be possible involving custom legalisation of FP_ROUND with an f16 target.

The bit manipulation logic on the trunc libcall return value is unnecessary (though just inefficient rather than incorrect), as the value should already be appropriately NaN-extended - this patch doesn't try to address this.

As can be seen in the test changes, the custom lowering for FP16_TO_FP means the libcall is no longer tail-callable.

I'm not totally sure this is the best way to fix this issue, so I'm posting this patch for initial feedback rather than suggesting it's directly ready for merging. Issues to be addressed:

- Redundant fmv.x.w and fmv.w.x pairs produced in FP16_TO_FP lowering.
- Missing STRICT variants.
- Ensure proper coverage of z{f,d}inx
- Merge half-fpext.ll and half-fptrunc.ll into half-conv.ll, adding to / refactoring existing RUN lines.



-------------- next part --------------
A non-text attachment was scrubbed...
Name: D151284.524989.patch
Type: text/x-patch
Size: 92832 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230524/35847d45/attachment-0001.bin>

More information about the llvm-commits mailing list