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

[Weiwei Li]

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 <liwei...@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqi...@iscas.ac.cn>
Reviewed-by: Daniel Henrique Barboza <dbarb...@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;