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

["Xiao Zeng" <zengxiao@eswincomputing.com>]

2024-05-09 04:01  Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
>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. 
Thanks

>
>I'll push the fixed version momentarily. 
Thanks

>
>Thanks again!
>
>jeff
> 
In the past few days, I have been on vacation. Okay, let's continue to push forward with bf16.

Thanks
Xiao Zeng