Re: [PATCH v1 1/1] RISC-V: Nan-box the result of movbf on soft-bf16

[Jeff Law <jeffreyalaw@gmail.com>]

On 5/7/24 6:38 PM, Xiao Zeng wrote:
> 1 This patch implements the Nan-box of bf16.
> 
> 2 Please refer to the Nan-box implementation of hf16 in:
> <https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=057dc349021660c40699fb5c98fd9cac8e168653>
> 
> 3 The discussion about Nan-box can be found on the website:
> <https://www.mail-archive.com/search?q=Nan-box+the+result+of+movhf+on+soft-fp16&l=gcc-patches%40gcc.gnu.org>
> 
> 4 Below test are passed for this patch
>          * The riscv fully regression test.
> 
> gcc/ChangeLog:
> 
> 	* config/riscv/riscv.cc (riscv_legitimize_move): Expand movbf
> 	with Nan-boxing value.
> 	* config/riscv/riscv.md (*movbf_softfloat_boxing): New pattern.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.target/riscv/_Bfloat16-nanboxing.c: New test.
> ---
>   gcc/config/riscv/riscv.cc                     | 51 ++++++++++---------
>   gcc/config/riscv/riscv.md                     | 12 ++++-
>   .../gcc.target/riscv/_Bfloat16-nanboxing.c    | 38 ++++++++++++++
>   3 files changed, 76 insertions(+), 25 deletions(-)
>   create mode 100644 gcc/testsuite/gcc.target/riscv/_Bfloat16-nanboxing.c
> 
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 545e68566dc..be2cb245733 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -3120,35 +3120,38 @@ riscv_legitimize_move (machine_mode mode, rtx dest, rtx src)

>   
> - if (TARGET_HARD_FLOAT
> -     && !TARGET_ZFHMIN && mode == HFmode
> -     && REG_P (dest) && FP_REG_P (REGNO (dest))
> -     && REG_P (src) && !FP_REG_P (REGNO (src))
> -     && can_create_pseudo_p ())
[ ... ]

> +  if (TARGET_HARD_FLOAT
> +      && ((!TARGET_ZFHMIN && mode == HFmode)
> +	  || (!TARGET_ZFBFMIN && mode == BFmode))
> +      && REG_P (dest) && FP_REG_P (REGNO (dest)) && REG_P (src)
> +      && !FP_REG_P (REGNO (src)) && can_create_pseudo_p ())

So there's a bit of gratutious rewriting going on here.  I realize you 
were fixing formatting problems (thanks!), but I don't see a need to 
rewriting the tests starting with REG_P.  I put those back in their 
original form with the whitespace fixes.

I'll push the fixed version momentarily.

Thanks again!

jeff