diff mbox series

[v2,1/6] target/riscv: Add properties for BF16 extensions

Message ID 20230615063302.102409-2-liweiwei@iscas.ac.cn
State New
Headers show
Series target/riscv: Add support for BF16 extensions | expand

Commit Message

Weiwei Li June 15, 2023, 6:32 a.m. UTC 
Add ext_zfbfmin/zvfbfmin/zvfbfwma properties.
Add require check for BF16 extensions.

Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 target/riscv/cpu.c     | 20 ++++++++++++++++++++
 target/riscv/cpu_cfg.h |  3 +++
 2 files changed, 23 insertions(+)

Comments

Rob Bradford June 15, 2023, 12:58 p.m. UTC | #1 
On Thu, 2023-06-15 at 14:32 +0800, Weiwei Li wrote:
> Add ext_zfbfmin/zvfbfmin/zvfbfwma properties.
> Add require check for BF16 extensions.
> 
> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>  target/riscv/cpu.c     | 20 ++++++++++++++++++++
>  target/riscv/cpu_cfg.h |  3 +++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 881bddf393..dc6b2f72f6 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1059,6 +1059,11 @@ void
> riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
>          return;
>      }
>  
> +    if (cpu->cfg.ext_zfbfmin && !riscv_has_ext(env, RVF)) {
> +        error_setg(errp, "Zfbfmin extension depends on F
> extension");
> +        return;
> +    }
> +
>      if (riscv_has_ext(env, RVD) && !riscv_has_ext(env, RVF)) {
>          error_setg(errp, "D extension requires F extension");
>          return;
> @@ -1109,6 +1114,21 @@ void
> riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
>          return;
>      }
>  
> +    if (cpu->cfg.ext_zvfbfmin && !cpu->cfg.ext_zfbfmin) {
> +        error_setg(errp, "Zvfbfmin extension depends on Zfbfmin
> extension");
> +        return;
> +    }
> +
> +    if (cpu->cfg.ext_zvfbfmin && !cpu->cfg.ext_zve32f) {
> +        error_setg(errp, "Zvfbfmin extension depends on Zve32f
> extension");
> +        return;
> +    }

I don't think this is correct - from the spec:

"This extension [Zvfbfmin] depends on the Zfbfmin extension and either
the "V" extension or the Zve32f embedded vector extension."

So this should be: 

+    if (cpu->cfg.ext_zvfbfmin && !(cpu->cfg.ext_zve32f || cpu-
>cfg.ext_v) {
+        error_setg(errp, "Zvfbfmin extension depends on Zve32f or V
extension");
+        return;
+    }

Cheers,

Rob

> +
> +    if (cpu->cfg.ext_zvfbfwma && !cpu->cfg.ext_zvfbfmin) {
> +        error_setg(errp, "Zvfbfwma extension depends on Zvfbfmin
> extension");
> +        return;
> +    }
> +
>      /* Set the ISA extensions, checks should have happened above */
>      if (cpu->cfg.ext_zhinx) {
>          cpu->cfg.ext_zhinxmin = true;
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index c4a627d335..7d16f32720 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -75,6 +75,7 @@ struct RISCVCPUConfig {
>      bool ext_svpbmt;
>      bool ext_zdinx;
>      bool ext_zawrs;
> +    bool ext_zfbfmin;
>      bool ext_zfh;
>      bool ext_zfhmin;
>      bool ext_zfinx;
> @@ -84,6 +85,8 @@ struct RISCVCPUConfig {
>      bool ext_zve64f;
>      bool ext_zve64d;
>      bool ext_zmmul;
> +    bool ext_zvfbfmin;
> +    bool ext_zvfbfwma;
>      bool ext_zvfh;
>      bool ext_zvfhmin;
>      bool ext_smaia;
Weiwei Li June 15, 2023, 1:14 p.m. UTC | #2 
On 2023/6/15 20:58, Rob Bradford wrote:
> On Thu, 2023-06-15 at 14:32 +0800, Weiwei Li wrote:
>> Add ext_zfbfmin/zvfbfmin/zvfbfwma properties.
>> Add require check for BF16 extensions.
>>
>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
>> Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>> ---
>>   target/riscv/cpu.c     | 20 ++++++++++++++++++++
>>   target/riscv/cpu_cfg.h |  3 +++
>>   2 files changed, 23 insertions(+)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 881bddf393..dc6b2f72f6 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -1059,6 +1059,11 @@ void
>> riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
>>           return;
>>       }
>>   
>> +    if (cpu->cfg.ext_zfbfmin && !riscv_has_ext(env, RVF)) {
>> +        error_setg(errp, "Zfbfmin extension depends on F
>> extension");
>> +        return;
>> +    }
>> +
>>       if (riscv_has_ext(env, RVD) && !riscv_has_ext(env, RVF)) {
>>           error_setg(errp, "D extension requires F extension");
>>           return;
>> @@ -1109,6 +1114,21 @@ void
>> riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
>>           return;
>>       }
>>   
>> +    if (cpu->cfg.ext_zvfbfmin && !cpu->cfg.ext_zfbfmin) {
>> +        error_setg(errp, "Zvfbfmin extension depends on Zfbfmin
>> extension");
>> +        return;
>> +    }
>> +
>> +    if (cpu->cfg.ext_zvfbfmin && !cpu->cfg.ext_zve32f) {
>> +        error_setg(errp, "Zvfbfmin extension depends on Zve32f
>> extension");
>> +        return;
>> +    }
> I don't think this is correct - from the spec:
>
> "This extension [Zvfbfmin] depends on the Zfbfmin extension and either
> the "V" extension or the Zve32f embedded vector extension."
>
> So this should be:
>
> +    if (cpu->cfg.ext_zvfbfmin && !(cpu->cfg.ext_zve32f || cpu-
>> cfg.ext_v) {
> +        error_setg(errp, "Zvfbfmin extension depends on Zve32f or V
> extension");
> +        return;
> +    }

Zve32f will be enabled when V is enabled. So we can simply check Zve32f 
here.

Regards,

Weiwei Li

> Cheers,
>
> Rob
>
>> +
>> +    if (cpu->cfg.ext_zvfbfwma && !cpu->cfg.ext_zvfbfmin) {
>> +        error_setg(errp, "Zvfbfwma extension depends on Zvfbfmin
>> extension");
>> +        return;
>> +    }
>> +
>>       /* Set the ISA extensions, checks should have happened above */
>>       if (cpu->cfg.ext_zhinx) {
>>           cpu->cfg.ext_zhinxmin = true;
>> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
>> index c4a627d335..7d16f32720 100644
>> --- a/target/riscv/cpu_cfg.h
>> +++ b/target/riscv/cpu_cfg.h
>> @@ -75,6 +75,7 @@ struct RISCVCPUConfig {
>>       bool ext_svpbmt;
>>       bool ext_zdinx;
>>       bool ext_zawrs;
>> +    bool ext_zfbfmin;
>>       bool ext_zfh;
>>       bool ext_zfhmin;
>>       bool ext_zfinx;
>> @@ -84,6 +85,8 @@ struct RISCVCPUConfig {
>>       bool ext_zve64f;
>>       bool ext_zve64d;
>>       bool ext_zmmul;
>> +    bool ext_zvfbfmin;
>> +    bool ext_zvfbfwma;
>>       bool ext_zvfh;
>>       bool ext_zvfhmin;
>>       bool ext_smaia;
Rob Bradford June 15, 2023, 3 p.m. UTC | #3 
On Thu, 2023-06-15 at 21:14 +0800, Weiwei Li wrote:
> 
> On 2023/6/15 20:58, Rob Bradford wrote:
> > On Thu, 2023-06-15 at 14:32 +0800, Weiwei Li wrote:
> > > Add ext_zfbfmin/zvfbfmin/zvfbfwma properties.
> > > Add require check for BF16 extensions.
> > > 
> > > Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> > > Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> > > Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> > > ---
> > >   target/riscv/cpu.c     | 20 ++++++++++++++++++++
> > >   target/riscv/cpu_cfg.h |  3 +++
> > >   2 files changed, 23 insertions(+)
> > > 
> > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > > index 881bddf393..dc6b2f72f6 100644
> > > --- a/target/riscv/cpu.c
> > > +++ b/target/riscv/cpu.c
> > > @@ -1059,6 +1059,11 @@ void
> > > riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
> > >           return;
> > >       }
> > >   
> > > +    if (cpu->cfg.ext_zfbfmin && !riscv_has_ext(env, RVF)) {
> > > +        error_setg(errp, "Zfbfmin extension depends on F
> > > extension");
> > > +        return;
> > > +    }
> > > +
> > >       if (riscv_has_ext(env, RVD) && !riscv_has_ext(env, RVF)) {
> > >           error_setg(errp, "D extension requires F extension");
> > >           return;
> > > @@ -1109,6 +1114,21 @@ void
> > > riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
> > >           return;
> > >       }
> > >   
> > > +    if (cpu->cfg.ext_zvfbfmin && !cpu->cfg.ext_zfbfmin) {
> > > +        error_setg(errp, "Zvfbfmin extension depends on Zfbfmin
> > > extension");
> > > +        return;
> > > +    }
> > > +
> > > +    if (cpu->cfg.ext_zvfbfmin && !cpu->cfg.ext_zve32f) {
> > > +        error_setg(errp, "Zvfbfmin extension depends on Zve32f
> > > extension");
> > > +        return;
> > > +    }
> > I don't think this is correct - from the spec:
> > 
> > "This extension [Zvfbfmin] depends on the Zfbfmin extension and
> > either
> > the "V" extension or the Zve32f embedded vector extension."
> > 
> > So this should be:
> > 
> > +    if (cpu->cfg.ext_zvfbfmin && !(cpu->cfg.ext_zve32f || cpu-
> > > cfg.ext_v) {
> > +        error_setg(errp, "Zvfbfmin extension depends on Zve32f or
> > V
> > extension");
> > +        return;
> > +    }
> 
> Zve32f will be enabled when V is enabled. So we can simply check
> Zve32f 
> here.

Great, thank you for the clarification - I missed that this this
enforced directly above.

Cheers,

Rob

> 
> Regards,
> 
> Weiwei Li
> 
> > Cheers,
> > 
> > Rob
> > 
> > > +
> > > +    if (cpu->cfg.ext_zvfbfwma && !cpu->cfg.ext_zvfbfmin) {
> > > +        error_setg(errp, "Zvfbfwma extension depends on Zvfbfmin
> > > extension");
> > > +        return;
> > > +    }
> > > +
> > >       /* Set the ISA extensions, checks should have happened
> > > above */
> > >       if (cpu->cfg.ext_zhinx) {
> > >           cpu->cfg.ext_zhinxmin = true;
> > > diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> > > index c4a627d335..7d16f32720 100644
> > > --- a/target/riscv/cpu_cfg.h
> > > +++ b/target/riscv/cpu_cfg.h
> > > @@ -75,6 +75,7 @@ struct RISCVCPUConfig {
> > >       bool ext_svpbmt;
> > >       bool ext_zdinx;
> > >       bool ext_zawrs;
> > > +    bool ext_zfbfmin;
> > >       bool ext_zfh;
> > >       bool ext_zfhmin;
> > >       bool ext_zfinx;
> > > @@ -84,6 +85,8 @@ struct RISCVCPUConfig {
> > >       bool ext_zve64f;
> > >       bool ext_zve64d;
> > >       bool ext_zmmul;
> > > +    bool ext_zvfbfmin;
> > > +    bool ext_zvfbfwma;
> > >       bool ext_zvfh;
> > >       bool ext_zvfhmin;
> > >       bool ext_smaia;
>
Alistair Francis July 3, 2023, 3:05 a.m. UTC | #4 
On Thu, Jun 15, 2023 at 4:34 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>
> Add ext_zfbfmin/zvfbfmin/zvfbfwma properties.
> Add require check for BF16 extensions.
>
> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu.c     | 20 ++++++++++++++++++++
>  target/riscv/cpu_cfg.h |  3 +++
>  2 files changed, 23 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 881bddf393..dc6b2f72f6 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1059,6 +1059,11 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
>          return;
>      }
>
> +    if (cpu->cfg.ext_zfbfmin && !riscv_has_ext(env, RVF)) {
> +        error_setg(errp, "Zfbfmin extension depends on F extension");
> +        return;
> +    }
> +
>      if (riscv_has_ext(env, RVD) && !riscv_has_ext(env, RVF)) {
>          error_setg(errp, "D extension requires F extension");
>          return;
> @@ -1109,6 +1114,21 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
>          return;
>      }
>
> +    if (cpu->cfg.ext_zvfbfmin && !cpu->cfg.ext_zfbfmin) {
> +        error_setg(errp, "Zvfbfmin extension depends on Zfbfmin extension");
> +        return;
> +    }
> +
> +    if (cpu->cfg.ext_zvfbfmin && !cpu->cfg.ext_zve32f) {
> +        error_setg(errp, "Zvfbfmin extension depends on Zve32f extension");
> +        return;
> +    }
> +
> +    if (cpu->cfg.ext_zvfbfwma && !cpu->cfg.ext_zvfbfmin) {
> +        error_setg(errp, "Zvfbfwma extension depends on Zvfbfmin extension");
> +        return;
> +    }
> +
>      /* Set the ISA extensions, checks should have happened above */
>      if (cpu->cfg.ext_zhinx) {
>          cpu->cfg.ext_zhinxmin = true;
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index c4a627d335..7d16f32720 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -75,6 +75,7 @@ struct RISCVCPUConfig {
>      bool ext_svpbmt;
>      bool ext_zdinx;
>      bool ext_zawrs;
> +    bool ext_zfbfmin;
>      bool ext_zfh;
>      bool ext_zfhmin;
>      bool ext_zfinx;
> @@ -84,6 +85,8 @@ struct RISCVCPUConfig {
>      bool ext_zve64f;
>      bool ext_zve64d;
>      bool ext_zmmul;
> +    bool ext_zvfbfmin;
> +    bool ext_zvfbfwma;
>      bool ext_zvfh;
>      bool ext_zvfhmin;
>      bool ext_smaia;
> --
> 2.25.1
>
>
diff mbox series

Patch

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 881bddf393..dc6b2f72f6 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1059,6 +1059,11 @@  void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
         return;
     }
 
+    if (cpu->cfg.ext_zfbfmin && !riscv_has_ext(env, RVF)) {
+        error_setg(errp, "Zfbfmin extension depends on F extension");
+        return;
+    }
+
     if (riscv_has_ext(env, RVD) && !riscv_has_ext(env, RVF)) {
         error_setg(errp, "D extension requires F extension");
         return;
@@ -1109,6 +1114,21 @@  void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
         return;
     }
 
+    if (cpu->cfg.ext_zvfbfmin && !cpu->cfg.ext_zfbfmin) {
+        error_setg(errp, "Zvfbfmin extension depends on Zfbfmin extension");
+        return;
+    }
+
+    if (cpu->cfg.ext_zvfbfmin && !cpu->cfg.ext_zve32f) {
+        error_setg(errp, "Zvfbfmin extension depends on Zve32f extension");
+        return;
+    }
+
+    if (cpu->cfg.ext_zvfbfwma && !cpu->cfg.ext_zvfbfmin) {
+        error_setg(errp, "Zvfbfwma extension depends on Zvfbfmin extension");
+        return;
+    }
+
     /* Set the ISA extensions, checks should have happened above */
     if (cpu->cfg.ext_zhinx) {
         cpu->cfg.ext_zhinxmin = true;
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index c4a627d335..7d16f32720 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -75,6 +75,7 @@  struct RISCVCPUConfig {
     bool ext_svpbmt;
     bool ext_zdinx;
     bool ext_zawrs;
+    bool ext_zfbfmin;
     bool ext_zfh;
     bool ext_zfhmin;
     bool ext_zfinx;
@@ -84,6 +85,8 @@  struct RISCVCPUConfig {
     bool ext_zve64f;
     bool ext_zve64d;
     bool ext_zmmul;
+    bool ext_zvfbfmin;
+    bool ext_zvfbfwma;
     bool ext_zvfh;
     bool ext_zvfhmin;
     bool ext_smaia;