2024-05-18 09:57  Jeff Law <jeffreya...@gmail.com> wrote:
>
>
>
>On 5/15/24 7:55 PM, Xiao Zeng wrote:
>> 1 According to unpriv-isa spec:
>> <https://github.com/riscv/riscv-isa-manual/releases/download/riscv-isa-release-221bd85-2024-05-14/unpriv-isa-asciidoc.pdf>
>>    1.1 "FMV.H.X moves the half-precision value encoded in IEEE 754-2008
>>    standard encoding from the lower 16 bits of integer register rs1
>>    to the floating-point register rd, NaN-boxing the result."
>>    1.2 "FMV.W.X moves the single-precision value encoded in IEEE 754-2008
>>    standard encoding from the lower 32 bits of integer register rs1
>>    to the floating-point register rd. The bits are not modified in the
>>    transfer, and in particular, the payloads of non-canonical NaNs are 
>>preserved."
>>
>> 2 When (!TARGET_ZFHMIN == true && TARGET_HARD_FLOAT == true), instruction 
>> needs
>> to be added to complete the Nan-box, as done in
>> "RISC-V: Nan-box the result of movhf on soft-fp16":
>> <https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=057dc349021660c40699fb5c98fd9cac8e168653>
>>
>> 3 Consider the "RISC-V: Nan-box the result of movbf on soft-bf16" in:
>> <https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=ce51e6727c9d69bbab0e766c449e60fd41f5f2f9>
>> It ignores that both hf16 and bf16 are 16bits floating-point.
>>
>> 4 zfbfmin -> zfhmin in:
>> <https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=35224ead63732a3550ba4b1332c06e9dc7999c31>
>>
>> gcc/ChangeLog:
>>
>> * config/riscv/riscv.cc (riscv_legitimize_move): Optimize movbf
>> with Nan-boxing value.
>> * config/riscv/riscv.md (*movhf_softfloat_boxing): Expand movbf
>> with Nan-boxing value.
>> (*mov<HFBF:mode>_softfloat_boxing): Ditto.
>> with Nan-boxing value.
>> (*movbf_softfloat_boxing): Delete abandon pattern.
>> ---
>>   gcc/config/riscv/riscv.cc | 15 +++++----------
>>   gcc/config/riscv/riscv.md | 19 +++++--------------
>>   2 files changed, 10 insertions(+), 24 deletions(-)
>>
>> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
>> index 4067505270e..04513537aad 100644
>> --- a/gcc/config/riscv/riscv.cc
>> +++ b/gcc/config/riscv/riscv.cc
>> @@ -3178,13 +3178,10 @@ riscv_legitimize_move (machine_mode mode, rtx dest, 
>> rtx src)
>>        (set (reg:SI/DI mask) (const_int -65536)
>>        (set (reg:SI/DI temp) (zero_extend:SI/DI (subreg:HI (reg:HF/BF src) 
>>0)))
>>        (set (reg:SI/DI temp) (ior:SI/DI (reg:SI/DI mask) (reg:SI/DI temp)))
>> -     (set (reg:HF/BF dest) (unspec:HF/BF[ (reg:SI/DI temp) ]
>> -        UNSPEC_FMV_SFP16_X/UNSPEC_FMV_SBF16_X))
>> -     */
>> +     (set (reg:HF/BF dest) (unspec:HF/BF[ (reg:SI/DI temp) ] 
>> UNSPEC_FMV_FP16_X))
>> +  */
>>  
>> -  if (TARGET_HARD_FLOAT
>> -      && ((!TARGET_ZFHMIN && mode == HFmode)
>> -      || (!TARGET_ZFBFMIN && mode == BFmode))
>> +  if (TARGET_HARD_FLOAT && !TARGET_ZFHMIN && (mode == HFmode || mode == 
>> BFmode)
>We generally prefer not to mix && and || operators on the same line.
>I'd suggest
>
>if (TARGET_HARD_FLOAT
>     && !TARGET_ZFHMIN
>     && (mode == HFmode || mode == BFmode)
>[ ... ] 
Fixed.

>
>
>> @@ -1959,23 +1958,15 @@
>>      (set_attr "type" "fmove,move,load,store,mtc,mfc")
>>      (set_attr "mode" "<MODE>")])
>>  
>> -(define_insn "*movhf_softfloat_boxing"
>> -  [(set (match_operand:HF 0 "register_operand"            "=f")
>> -        (unspec:HF [(match_operand:X 1 "register_operand" " r")] 
>> UNSPEC_FMV_SFP16_X))]
>> +(define_insn "*mov<HFBF:mode>_softfloat_boxing"
>> +  [(set (match_operand:HFBF 0 "register_operand"        "=f")
>> +        (unspec:HFBF [(match_operand:X 1 "register_operand" " r")]
>> +    UNSPEC_FMV_FP16_X))]
>>     "!TARGET_ZFHMIN"
>I think the linter complained about having 8 spaces instead of a tab in
>one of the lines above. 
Fixed.

>
>With those fixes, this is fine for the trunk.
>
>jeff
Thanks
Xiao Zeng

Reply via email to