[PATCH] RISC-V: Fix "Nan-box the result of movbf on soft-bf16"

[PATCH] RISC-V: Fix "Nan-box the result of movbf on soft-bf16"

[Xiao Zeng]

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)
       && REG_P (dest) && FP_REG_P (REGNO (dest))
       && REG_P (src) && !FP_REG_P (REGNO (src))
       && can_create_pseudo_p ())
@@ -3199,10 +3196,8 @@ riscv_legitimize_move (machine_mode mode, rtx dest, rtx src)
       else
 	emit_insn (gen_iordi3 (temp, mask, temp));
 
-      riscv_emit_move (dest,
-		       gen_rtx_UNSPEC (mode, gen_rtvec (1, temp),
-				       mode == HFmode ? UNSPEC_FMV_SFP16_X
-						      : UNSPEC_FMV_SBF16_X));
+      riscv_emit_move (dest, gen_rtx_UNSPEC (mode, gen_rtvec (1, temp),
+					     UNSPEC_FMV_FP16_X));
 
       return true;
     }
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index ee15c63db10..4734bbc17df 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -87,8 +87,7 @@
   UNSPEC_STRLEN
 
   ;; Workaround for HFmode and BFmode without hardware extension
-  UNSPEC_FMV_SFP16_X
-  UNSPEC_FMV_SBF16_X
+  UNSPEC_FMV_FP16_X
 
   ;; XTheadFmv moves
   UNSPEC_XTHEADFMV
@@ -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"
   "fmv.w.x\t%0,%1"
   [(set_attr "type" "fmove")
    (set_attr "mode" "SF")])
 
-(define_insn "*movbf_softfloat_boxing"
-  [(set (match_operand:BF 0 "register_operand"		  "=f")
-	(unspec:BF [(match_operand:X 1 "register_operand" " r")]
-	 UNSPEC_FMV_SBF16_X))]
-  "!TARGET_ZFBFMIN"
-  "fmv.w.x\t%0,%1"
-  [(set_attr "type" "fmove")
-   (set_attr "mode" "SF")])
-
 ;;
 ;;  ....................
 ;;
-- 
2.17.1

Re: [PATCH] RISC-V: Fix "Nan-box the result of movbf on soft-bf16"

[Jeff Law]

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)
[ ... ]


> @@ -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.

With those fixes, this is fine for the trunk.

jeff

[COMMITTED] RISC-V: Fix "Nan-box the result of movbf on soft-bf16"

[Xiao Zeng]

2024-05-18 09:57  Jeff Law <jeffreyalaw@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