[PATCH v2 0/7] target/riscv: NaN-boxing for multiple precison

Richard Henderson posted 7 patches 3 years, 9 months ago
Test docker-quick@centos7 failed
Test docker-mingw@fedora failed
Test checkpatch failed
Test FreeBSD failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200724002807.441147-1-richard.henderson@linaro.org
Maintainers: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Palmer Dabbelt <palmer@dabbelt.com>, Sagar Karandikar <sagark@eecs.berkeley.edu>, Alistair Francis <Alistair.Francis@wdc.com>
target/riscv/internals.h                |  16 ++++
target/riscv/fpu_helper.c               | 102 ++++++++++++++++--------
target/riscv/insn_trans/trans_rvd.inc.c |   8 +-
target/riscv/insn_trans/trans_rvf.inc.c |  99 ++++++++++++++---------
target/riscv/translate.c                |  29 +++++++
5 files changed, 178 insertions(+), 76 deletions(-)
[PATCH v2 0/7] target/riscv: NaN-boxing for multiple precison
Posted by Richard Henderson 3 years, 9 months ago
This is my take on Liu Zhiwei's patch set:
https://patchew.org/QEMU/20200626205917.4545-1-zhiwei_liu@c-sky.com

This differs from Zhiwei's v1 in:

 * If a helper is involved, the helper does the boxing and unboxing.

 * Which leaves only LDW and FSGN*.S as the only instructions that
   are expanded inline which need to handle nanboxing.

 * All mention of RVD is dropped vs boxing.  This means that an
   RVF-only cpu will still generate and check nanboxes into the
   64-bit cpu_fpu slots.  There should be no way an RVF-only cpu
   can generate an unboxed cpu_fpu value.

   This choice is made to speed up the common case: RVF+RVD, so
   that we do not have to check whether RVD is enabled.

 * The translate.c primitives take TCGv values rather than fpu
   regno, which will make it possible to use them with RVV,
   since v0.9 does proper nanboxing.

 * I have adjusted the current naming to be float32 specific ("*_s"),
   to avoid confusion with the float16 data type supported by RVV.


r~


LIU Zhiwei (2):
  target/riscv: Clean up fmv.w.x
  target/riscv: check before allocating TCG temps

Richard Henderson (5):
  target/riscv: Generate nanboxed results from fp helpers
  target/riscv: Generalize gen_nanbox_fpr to gen_nanbox_s
  target/riscv: Generate nanboxed results from trans_rvf.inc.c
  target/riscv: Check nanboxed inputs to fp helpers
  target/riscv: Check nanboxed inputs in trans_rvf.inc.c

 target/riscv/internals.h                |  16 ++++
 target/riscv/fpu_helper.c               | 102 ++++++++++++++++--------
 target/riscv/insn_trans/trans_rvd.inc.c |   8 +-
 target/riscv/insn_trans/trans_rvf.inc.c |  99 ++++++++++++++---------
 target/riscv/translate.c                |  29 +++++++
 5 files changed, 178 insertions(+), 76 deletions(-)

-- 
2.25.1


Re: [PATCH v2 0/7] target/riscv: NaN-boxing for multiple precison
Posted by Alistair Francis 3 years, 9 months ago
On Thu, Jul 23, 2020 at 5:28 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> This is my take on Liu Zhiwei's patch set:
> https://patchew.org/QEMU/20200626205917.4545-1-zhiwei_liu@c-sky.com
>
> This differs from Zhiwei's v1 in:
>
>  * If a helper is involved, the helper does the boxing and unboxing.
>
>  * Which leaves only LDW and FSGN*.S as the only instructions that
>    are expanded inline which need to handle nanboxing.
>
>  * All mention of RVD is dropped vs boxing.  This means that an
>    RVF-only cpu will still generate and check nanboxes into the
>    64-bit cpu_fpu slots.  There should be no way an RVF-only cpu
>    can generate an unboxed cpu_fpu value.
>
>    This choice is made to speed up the common case: RVF+RVD, so
>    that we do not have to check whether RVD is enabled.
>
>  * The translate.c primitives take TCGv values rather than fpu
>    regno, which will make it possible to use them with RVV,
>    since v0.9 does proper nanboxing.
>
>  * I have adjusted the current naming to be float32 specific ("*_s"),
>    to avoid confusion with the float16 data type supported by RVV.

Thanks Richard. As Zhiwei has reviewed all of these I have applied
them to the riscv-to-apply.next tree for 5.2.

Alistair

>
>
> r~
>
>
> LIU Zhiwei (2):
>   target/riscv: Clean up fmv.w.x
>   target/riscv: check before allocating TCG temps
>
> Richard Henderson (5):
>   target/riscv: Generate nanboxed results from fp helpers
>   target/riscv: Generalize gen_nanbox_fpr to gen_nanbox_s
>   target/riscv: Generate nanboxed results from trans_rvf.inc.c
>   target/riscv: Check nanboxed inputs to fp helpers
>   target/riscv: Check nanboxed inputs in trans_rvf.inc.c
>
>  target/riscv/internals.h                |  16 ++++
>  target/riscv/fpu_helper.c               | 102 ++++++++++++++++--------
>  target/riscv/insn_trans/trans_rvd.inc.c |   8 +-
>  target/riscv/insn_trans/trans_rvf.inc.c |  99 ++++++++++++++---------
>  target/riscv/translate.c                |  29 +++++++
>  5 files changed, 178 insertions(+), 76 deletions(-)
>
> --
> 2.25.1
>

Re: [PATCH v2 0/7] target/riscv: NaN-boxing for multiple precison
Posted by LIU Zhiwei 3 years, 9 months ago

On 2020/7/24 8:28, Richard Henderson wrote:
> This is my take on Liu Zhiwei's patch set:
> https://patchew.org/QEMU/20200626205917.4545-1-zhiwei_liu@c-sky.com
>
> This differs from Zhiwei's v1 in:
>
>   * If a helper is involved, the helper does the boxing and unboxing.
>
>   * Which leaves only LDW and FSGN*.S as the only instructions that
>     are expanded inline which need to handle nanboxing.
>
>   * All mention of RVD is dropped vs boxing.  This means that an
>     RVF-only cpu will still generate and check nanboxes into the
>     64-bit cpu_fpu slots.  There should be no way an RVF-only cpu
>     can generate an unboxed cpu_fpu value.
>
>     This choice is made to speed up the common case: RVF+RVD, so
>     that we do not have to check whether RVD is enabled.
>
>   * The translate.c primitives take TCGv values rather than fpu
>     regno, which will make it possible to use them with RVV,
>     since v0.9 does proper nanboxing.
Agree.

And I think this patch set should be applied  if possible, because it is 
bug fix.
>   * I have adjusted the current naming to be float32 specific ("*_s"),
>     to avoid confusion with the float16 data type supported by RVV.
It's OK.

A more general function with flen is better in my opinion. So that it 
can be used
everywhere, both in scalar and vector instructions, even the future fp16 or
bf16 instructions.

Zhiwei
>
> r~
>
>
> LIU Zhiwei (2):
>    target/riscv: Clean up fmv.w.x
>    target/riscv: check before allocating TCG temps
>
> Richard Henderson (5):
>    target/riscv: Generate nanboxed results from fp helpers
>    target/riscv: Generalize gen_nanbox_fpr to gen_nanbox_s
>    target/riscv: Generate nanboxed results from trans_rvf.inc.c
>    target/riscv: Check nanboxed inputs to fp helpers
>    target/riscv: Check nanboxed inputs in trans_rvf.inc.c
>
>   target/riscv/internals.h                |  16 ++++
>   target/riscv/fpu_helper.c               | 102 ++++++++++++++++--------
>   target/riscv/insn_trans/trans_rvd.inc.c |   8 +-
>   target/riscv/insn_trans/trans_rvf.inc.c |  99 ++++++++++++++---------
>   target/riscv/translate.c                |  29 +++++++
>   5 files changed, 178 insertions(+), 76 deletions(-)
>