mbox series

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

Message ID 20200724002807.441147-1-richard.henderson@linaro.org
Headers show
Series target/riscv: NaN-boxing for multiple precison | expand

Message

Richard Henderson July 24, 2020, 12:28 a.m. UTC
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

Comments

LIU Zhiwei July 24, 2020, 2:31 a.m. UTC | #1
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(-)

>
Alistair Francis July 27, 2020, 11:37 p.m. UTC | #2
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

>