| Message ID | 20240516015504.38411-1-zengxiao@eswincomputing.com |
|---|---|
| State | New |
| Headers | show |
| Series | RISC-V: Fix "Nan-box the result of movbf on soft-bf16" | expand |
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
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
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")]) - ;; ;; .................... ;;