| Message ID | 00e7b1c6060dab32ac7d49813b1ca84d3eb63298.1652583332.git.research_trasio@irq.a4lg.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/5] target/riscv: Fix coding style on "G" expansion | expand |
在 2022/5/15 上午10:56, Tsukasa OI 写道: > QEMU allowed inconsistent configurations that made floating point > arithmetic effectively unusable. > > This commit adds certain checks for consistent FP arithmetic: > > - F requires Zicsr > - Zfinx requires Zicsr > - Zfh/Zfhmin require F > - D requires F > - V requires D Why 'V requires D'? I know partial vector instructions require D, However, I think they just like c.fsd instruction requires D, not 'C requires D' or 'D requires C'. Is there any rvv spec change I don't know? Regards. Weiwei Li > > Because F/D/Zicsr are enabled by default (and an error will not occur unless > we manually disable one or more of prerequisites), this commit just enforces > the user to give consistent combinations. > > Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com> > --- > target/riscv/cpu.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 0854ca9103..f910a41407 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -610,11 +610,36 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) > cpu->cfg.ext_ifencei = true; > } > > + if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) { > + error_setg(errp, "F extension requires Zicsr"); > + return; > + } > + > + if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) { > + error_setg(errp, "Zfh/Zfhmin extensions require F extension"); > + return; > + } > + > + if (cpu->cfg.ext_d && !cpu->cfg.ext_f) { > + error_setg(errp, "D extension requires F extension"); > + return; > + } > + > + if (cpu->cfg.ext_v && !cpu->cfg.ext_d) { > + error_setg(errp, "V extension requires D extension"); > + return; > + } > + > if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx || > cpu->cfg.ext_zhinxmin) { > cpu->cfg.ext_zfinx = true; > } > > + if (cpu->cfg.ext_zfinx && !cpu->cfg.ext_icsr) { > + error_setg(errp, "Zfinx extension requires Zicsr"); > + return; > + } > + > if (cpu->cfg.ext_zk) { > cpu->cfg.ext_zkn = true; > cpu->cfg.ext_zkr = true;
On 2022/05/15 23:37, Weiwei Li wrote: > > 在 2022/5/15 上午10:56, Tsukasa OI 写道: >> QEMU allowed inconsistent configurations that made floating point >> arithmetic effectively unusable. >> >> This commit adds certain checks for consistent FP arithmetic: >> >> - F requires Zicsr >> - Zfinx requires Zicsr >> - Zfh/Zfhmin require F >> - D requires F >> - V requires D > > Why 'V requires D'? I know partial vector instructions require D, However, I think they just like c.fsd > > instruction requires D, not 'C requires D' or 'D requires C'. Is there any rvv spec change I don't know? I thought it was crystal-clear. RISC-V "V" Vector Extension Version 1.0 (riscv-v-spec-1.0.pdf) 18.3. V: Vector Extension for Application Processors Page 94: > The V extension requires the scalar processor implements the F and D > extensions, and implements all vector floating-point instructions > (Section Vector Floating-Point Instructions) for floating-point operands > with EEW=32 or EEW=64 (including widening instructions and conversions > between FP32 and FP64). c.f. <https://github.com/riscv/riscv-v-spec/releases/tag/v1.0> Thanks, Tsukasa > > Regards. > > Weiwei Li > >> >> Because F/D/Zicsr are enabled by default (and an error will not occur unless >> we manually disable one or more of prerequisites), this commit just enforces >> the user to give consistent combinations. >> >> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com> >> --- >> target/riscv/cpu.c | 25 +++++++++++++++++++++++++ >> 1 file changed, 25 insertions(+) >> >> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c >> index 0854ca9103..f910a41407 100644 >> --- a/target/riscv/cpu.c >> +++ b/target/riscv/cpu.c >> @@ -610,11 +610,36 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) >> cpu->cfg.ext_ifencei = true; >> } >> + if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) { >> + error_setg(errp, "F extension requires Zicsr"); >> + return; >> + } >> + >> + if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) { >> + error_setg(errp, "Zfh/Zfhmin extensions require F extension"); >> + return; >> + } >> + >> + if (cpu->cfg.ext_d && !cpu->cfg.ext_f) { >> + error_setg(errp, "D extension requires F extension"); >> + return; >> + } >> + >> + if (cpu->cfg.ext_v && !cpu->cfg.ext_d) { >> + error_setg(errp, "V extension requires D extension"); >> + return; >> + } >> + >> if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx || >> cpu->cfg.ext_zhinxmin) { >> cpu->cfg.ext_zfinx = true; >> } >> + if (cpu->cfg.ext_zfinx && !cpu->cfg.ext_icsr) { >> + error_setg(errp, "Zfinx extension requires Zicsr"); >> + return; >> + } >> + >> if (cpu->cfg.ext_zk) { >> cpu->cfg.ext_zkn = true; >> cpu->cfg.ext_zkr = true; >
在 2022/5/15 下午10:45, Tsukasa OI 写道: > On 2022/05/15 23:37, Weiwei Li wrote: >> 在 2022/5/15 上午10:56, Tsukasa OI 写道: >>> QEMU allowed inconsistent configurations that made floating point >>> arithmetic effectively unusable. >>> >>> This commit adds certain checks for consistent FP arithmetic: >>> >>> - F requires Zicsr >>> - Zfinx requires Zicsr >>> - Zfh/Zfhmin require F >>> - D requires F >>> - V requires D >> Why 'V requires D'? I know partial vector instructions require D, However, I think they just like c.fsd >> >> instruction requires D, not 'C requires D' or 'D requires C'. Is there any rvv spec change I don't know? > I thought it was crystal-clear. > > RISC-V "V" Vector Extension Version 1.0 (riscv-v-spec-1.0.pdf) > 18.3. V: Vector Extension for Application Processors > Page 94: > >> The V extension requires the scalar processor implements the F and D >> extensions, and implements all vector floating-point instructions >> (Section Vector Floating-Point Instructions) for floating-point operands >> with EEW=32 or EEW=64 (including widening instructions and conversions >> between FP32 and FP64). > c.f. <https://github.com/riscv/riscv-v-spec/releases/tag/v1.0> > > Thanks, > > Tsukasa OK. Thanks. Regards, Weiwei Li > >> Regards. >> >> Weiwei Li >> >>> Because F/D/Zicsr are enabled by default (and an error will not occur unless >>> we manually disable one or more of prerequisites), this commit just enforces >>> the user to give consistent combinations. >>> >>> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com> >>> --- >>> target/riscv/cpu.c | 25 +++++++++++++++++++++++++ >>> 1 file changed, 25 insertions(+) >>> >>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c >>> index 0854ca9103..f910a41407 100644 >>> --- a/target/riscv/cpu.c >>> +++ b/target/riscv/cpu.c >>> @@ -610,11 +610,36 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) >>> cpu->cfg.ext_ifencei = true; >>> } >>> + if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) { >>> + error_setg(errp, "F extension requires Zicsr"); >>> + return; >>> + } >>> + >>> + if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) { >>> + error_setg(errp, "Zfh/Zfhmin extensions require F extension"); >>> + return; >>> + } >>> + >>> + if (cpu->cfg.ext_d && !cpu->cfg.ext_f) { >>> + error_setg(errp, "D extension requires F extension"); >>> + return; >>> + } >>> + >>> + if (cpu->cfg.ext_v && !cpu->cfg.ext_d) { >>> + error_setg(errp, "V extension requires D extension"); >>> + return; >>> + } >>> + >>> if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx || >>> cpu->cfg.ext_zhinxmin) { >>> cpu->cfg.ext_zfinx = true; >>> } >>> + if (cpu->cfg.ext_zfinx && !cpu->cfg.ext_icsr) { >>> + error_setg(errp, "Zfinx extension requires Zicsr"); >>> + return; >>> + } >>> + >>> if (cpu->cfg.ext_zk) { >>> cpu->cfg.ext_zkn = true; >>> cpu->cfg.ext_zkr = true;
On Sun, May 15, 2022 at 12:56 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote: > > QEMU allowed inconsistent configurations that made floating point > arithmetic effectively unusable. > > This commit adds certain checks for consistent FP arithmetic: > > - F requires Zicsr > - Zfinx requires Zicsr > - Zfh/Zfhmin require F > - D requires F > - V requires D > > Because F/D/Zicsr are enabled by default (and an error will not occur unless > we manually disable one or more of prerequisites), this commit just enforces > the user to give consistent combinations. > > Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > target/riscv/cpu.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 0854ca9103..f910a41407 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -610,11 +610,36 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) > cpu->cfg.ext_ifencei = true; > } > > + if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) { > + error_setg(errp, "F extension requires Zicsr"); > + return; > + } > + > + if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) { > + error_setg(errp, "Zfh/Zfhmin extensions require F extension"); > + return; > + } > + > + if (cpu->cfg.ext_d && !cpu->cfg.ext_f) { > + error_setg(errp, "D extension requires F extension"); > + return; > + } > + > + if (cpu->cfg.ext_v && !cpu->cfg.ext_d) { > + error_setg(errp, "V extension requires D extension"); > + return; > + } > + > if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx || > cpu->cfg.ext_zhinxmin) { > cpu->cfg.ext_zfinx = true; > } > > + if (cpu->cfg.ext_zfinx && !cpu->cfg.ext_icsr) { > + error_setg(errp, "Zfinx extension requires Zicsr"); > + return; > + } > + > if (cpu->cfg.ext_zk) { > cpu->cfg.ext_zkn = true; > cpu->cfg.ext_zkr = true; > -- > 2.34.1 >
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 0854ca9103..f910a41407 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -610,11 +610,36 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) cpu->cfg.ext_ifencei = true; } + if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) { + error_setg(errp, "F extension requires Zicsr"); + return; + } + + if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) { + error_setg(errp, "Zfh/Zfhmin extensions require F extension"); + return; + } + + if (cpu->cfg.ext_d && !cpu->cfg.ext_f) { + error_setg(errp, "D extension requires F extension"); + return; + } + + if (cpu->cfg.ext_v && !cpu->cfg.ext_d) { + error_setg(errp, "V extension requires D extension"); + return; + } + if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx || cpu->cfg.ext_zhinxmin) { cpu->cfg.ext_zfinx = true; } + if (cpu->cfg.ext_zfinx && !cpu->cfg.ext_icsr) { + error_setg(errp, "Zfinx extension requires Zicsr"); + return; + } + if (cpu->cfg.ext_zk) { cpu->cfg.ext_zkn = true; cpu->cfg.ext_zkr = true;
QEMU allowed inconsistent configurations that made floating point arithmetic effectively unusable. This commit adds certain checks for consistent FP arithmetic: - F requires Zicsr - Zfinx requires Zicsr - Zfh/Zfhmin require F - D requires F - V requires D Because F/D/Zicsr are enabled by default (and an error will not occur unless we manually disable one or more of prerequisites), this commit just enforces the user to give consistent combinations. Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com> --- target/riscv/cpu.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)