Merge pull request #1701 from riscv-software-src/zvl_zve

Correctly determine vector capability from v/zve/zvl ISA strings, remove --varch

.github/workflows/debug-smoke.yml

@@ -47,7 +47,7 @@ jobs:
         run: |
           git clone --recurse-submodules https://github.com/riscv-software-src/riscv-tests.git
           cd riscv-tests
-          git checkout bd0a19c136927eaa3b7296a591a896c141affb6b
+          git checkout 00ab5f0dd4cf56b5a0551bc5adedf60c765d0c66
 
       - name: Run Tests
         run: |

config.h.in

@@ -12,9 +12,6 @@
 /* Default value for --priv switch */
 #undef DEFAULT_PRIV
 
-/* Default value for --varch switch */
-#undef DEFAULT_VARCH
-
 /* Define if subproject MCPPBS_SPROJ_NORM is enabled */
 #undef DISASM_ENABLED
 

configure

@@ -739,7 +739,6 @@ with_boost_asio
 with_boost_regex
 with_isa
 with_priv
-with_varch
 with_target
 enable_dual_endian
 '
@@ -1407,8 +1406,6 @@ Optional Packages:
   --with-isa=RV64IMAFDC_zicntr_zihpm
                           Sets the default RISC-V ISA
   --with-priv=MSU         Sets the default RISC-V privilege modes supported
-  --with-varch=vlen:128,elen:64
-                          Sets the default vector config
   --with-target=riscv64-unknown-elf
                           Sets the default target config
 
@@ -6593,20 +6590,6 @@ fi
 
 
 
-# Check whether --with-varch was given.
-if test ${with_varch+y}
-then :
-  withval=$with_varch;
-printf "%s\n" "#define DEFAULT_VARCH \"$withval\"" >>confdefs.h
-
-else $as_nop
-
-printf "%s\n" "#define DEFAULT_VARCH \"vlen:128,elen:64\"" >>confdefs.h
-
-fi
-
-
-
 # Check whether --with-target was given.
 if test ${with_target+y}
 then :

disasm/disasm.cc

@@ -1455,7 +1455,7 @@ void disassembler_t::add_instructions(const isa_parser_t* isa)
     DISASM_INSN("cm.jalt", cm_jalt, 0, {&rvcm_jt_index});
   }
 
-  if (isa->extension_enabled('V')) {
+  if (isa->has_any_vector()) {
     DISASM_INSN("vsetivli", vsetivli, 0, {&xrd, &zimm5, &v_vtype});
     DISASM_INSN("vsetvli", vsetvli, 0, {&xrd, &xrs1, &v_vtype});
     DEFINE_RTYPE(vsetvl);

disasm/isa_parser.cc

@@ -38,6 +38,11 @@ isa_parser_t::isa_parser_t(const char* str, const char *priv)
   else
     bad_isa_string(str, "ISA strings must begin with RV32 or RV64");
 
+  vlen = 0;
+  elen = 0;
+  zvf = false;
+  zvd = false;
+
   switch (isa_string[4]) {
     case 'g':
       // G = IMAFD_Zicsr_Zifencei, but Spike includes the latter two
@@ -70,7 +75,8 @@ isa_parser_t::isa_parser_t(const char* str, const char *priv)
     }
 
     switch (*p) {
-      case 'v': // even rv32iv implies double float
+      case 'v': vlen = 128; elen = 64; zvf = true; zvd = true;
+                // even rv32iv implies double float
       case 'q': extension_table['D'] = true;
                 // Fall through
       case 'd': extension_table['F'] = true;
@@ -95,9 +101,6 @@ isa_parser_t::isa_parser_t(const char* str, const char *priv)
       if (ext_str == "zfh")
         extension_table[EXT_ZFH] = true;
     } else if (ext_str == "zvfh" || ext_str == "zvfhmin") {
-      if (!extension_table['V'])
-        bad_isa_string(str, ("'" + ext_str + "' extension requires 'V'").c_str());
-
       extension_table[EXT_ZVFHMIN] = true;
 
       if (ext_str == "zvfh") {
@@ -314,6 +317,35 @@ isa_parser_t::isa_parser_t(const char* str, const char *priv)
       extension_table[EXT_ZICFILP] = true;
     } else if (ext_str == "zicfiss") {
       extension_table[EXT_ZICFISS] = true;
+    } else if (ext_str.substr(0, 3) == "zvl") {
+      reg_t new_vlen;
+      try {
+        new_vlen = std::stol(ext_str.substr(3, ext_str.size() - 4));
+      } catch (std::logic_error& e) {
+        new_vlen = 0;
+      }
+      if ((new_vlen & (new_vlen - 1)) != 0 || new_vlen < 32)
+        bad_isa_string(str, ("Invalid Zvl string: " + ext_str).c_str());
+      vlen = std::max(vlen, new_vlen);
+    } else if (ext_str.substr(0, 3) == "zve") {
+      reg_t new_elen;
+      try {
+        new_elen = std::stol(ext_str.substr(3, ext_str.size() - 4));
+      } catch (std::logic_error& e) {
+        new_elen = 0;
+      }
+      if (ext_str.substr(5) == "d") {
+        zvd |= true; zvf |= true;
+      } else if (ext_str.substr(5) == "f") {
+        zvf |= true;
+      } else if (ext_str.substr(5) == "x") {
+        /* do nothing */
+      } else {
+        new_elen = 0;
+      }
+      if (new_elen != 32 && new_elen != 64)
+        bad_isa_string(str, ("Invalid Zve string: " + ext_str).c_str());
+      elen = std::max(elen, new_elen);
     } else if (ext_str[0] == 'x') {
       extension_table['X'] = true;
       if (ext_str.size() == 1) {
@@ -424,6 +456,31 @@ isa_parser_t::isa_parser_t(const char* str, const char *priv)
 		     "extensions are incompatible with WORDS_BIGENDIAN setups.");
   }
 #endif
+
+  if (vlen > 4096) {
+    bad_isa_string(str, "Spike does not currently support VLEN > 4096b");
+  }
+
+  if ((vlen != 0) ^ (elen != 0)) {
+    bad_isa_string(str, "Invalid Zvl/Zve configuration");
+  }
+
+  if (extension_table[EXT_ZVFHMIN] && (vlen == 0 || elen == 0 || !zvf)) {
+    bad_isa_string(str, "'Zvfhmin' extension requires Zve32f");
+  }
+
+  if (extension_table[EXT_ZVFH] && (vlen == 0 || elen == 0 || !zvf || !extension_table[EXT_ZVFHMIN])) {
+    bad_isa_string(str, "'Zvfh' extension requires Zve32f and 'Zvfhmin'");
+  }
+
+  if (zvd && !extension_table['D'] && elen < 64) {
+    bad_isa_string(str, "'ZveXXd' extension requires D");
+  }
+
+  if (zvf && !extension_table['F']) {
+    bad_isa_string(str, "'ZveXXf' extension requires F");
+  }
+
   std::string lowercase = strtolower(priv);
   bool user = false, supervisor = false;
 

riscv/cfg.cc

@@ -35,7 +35,6 @@ cfg_t::cfg_t()
   bootargs         = nullptr;
   isa              = DEFAULT_ISA;
   priv             = DEFAULT_PRIV;
-  varch            = DEFAULT_VARCH;
   misaligned       = false;
   endianness       = endianness_little;
   pmpregions       = 16;

riscv/cfg.h

@@ -67,7 +67,6 @@ public:
   const char *            bootargs;
   const char *            isa;
   const char *            priv;
-  const char *            varch;
   bool                    misaligned;
   endianness_t            endianness;
   reg_t                   pmpregions;

riscv/csrs.cc

@@ -411,9 +411,9 @@ base_status_csr_t::base_status_csr_t(processor_t* const proc, const reg_t addr):
 reg_t base_status_csr_t::compute_sstatus_write_mask() const noexcept {
   // If a configuration has FS bits, they will always be accessible no
   // matter the state of misa.
-  const bool has_fs = (proc->extension_enabled('S') || proc->extension_enabled('F')
-              || proc->extension_enabled('V')) && !proc->extension_enabled(EXT_ZFINX);
-  const bool has_vs = proc->extension_enabled('V');
+  const bool has_fs = (proc->extension_enabled('S') || proc->extension_enabled('F')) && !proc->extension_enabled(EXT_ZFINX);
+  // Implementations w/o V may still have mstatus.vs,
+  const bool has_vs = proc->any_vector_extensions();
   return 0
     | (proc->extension_enabled('S') ? (SSTATUS_SIE | SSTATUS_SPIE | SSTATUS_SPP) : 0)
     | (has_page ? (SSTATUS_SUM | SSTATUS_MXR) : 0)
@@ -1429,8 +1429,6 @@ vector_csr_t::vector_csr_t(processor_t* const proc, const reg_t addr, const reg_
 
 void vector_csr_t::verify_permissions(insn_t insn, bool write) const {
   require_vector_vs;
-  if (!proc->extension_enabled('V'))
-    throw trap_illegal_instruction(insn.bits());
   basic_csr_t::verify_permissions(insn, write);
 }
 
@@ -1452,8 +1450,6 @@ vxsat_csr_t::vxsat_csr_t(processor_t* const proc, const reg_t addr):
 
 void vxsat_csr_t::verify_permissions(insn_t insn, bool write) const {
   require_vector_vs;
-  if (!proc->extension_enabled('V'))
-    throw trap_illegal_instruction(insn.bits());
   masked_csr_t::verify_permissions(insn, write);
 }
 

riscv/decode_macros.h

@@ -171,7 +171,6 @@ static inline bool is_aligned(const unsigned val, const unsigned pos)
 #define require_vector(alu) \
   do { \
     require_vector_vs; \
-    require_extension('V'); \
     require(!P.VU.vill); \
     if (alu && !P.VU.vstart_alu) \
       require(P.VU.vstart->read() == 0); \
@@ -181,7 +180,6 @@ static inline bool is_aligned(const unsigned val, const unsigned pos)
 #define require_vector_novtype(is_log) \
   do { \
     require_vector_vs; \
-    require_extension('V'); \
     if (is_log) \
       WRITE_VSTATUS; \
     dirty_vs_state; \

riscv/isa_parser.h

@@ -102,12 +102,17 @@ public:
   unsigned get_max_xlen() const { return max_xlen; }
   reg_t get_max_isa() const { return max_isa; }
   std::string get_isa_string() const { return isa_string; }
+  reg_t get_vlen() const { return vlen; }
+  reg_t get_elen() const { return elen; }
+  bool get_zvf() const { return zvf; }
+  bool get_zvd() const { return zvd; }
   bool extension_enabled(unsigned char ext) const {
     return extension_enabled(isa_extension_t(ext));
   }
   bool extension_enabled(isa_extension_t ext) const {
     return extension_table[ext];
   }
+  bool has_any_vector() const { return vlen > 0; }
 
   std::bitset<NUM_ISA_EXTENSIONS> get_extension_table() const { return extension_table; }
 
@@ -116,6 +121,10 @@ public:
 protected:
   unsigned max_xlen;
   reg_t max_isa;
+  reg_t vlen;
+  reg_t elen;
+  bool zvf;
+  bool zvd;
   std::bitset<NUM_ISA_EXTENSIONS> extension_table;
   std::string isa_string;
   std::set<std::string> extensions;

riscv/processor.cc

@@ -44,7 +44,7 @@ processor_t::processor_t(const isa_parser_t *isa, const cfg_t *cfg,
   TM.proc = this;
 
 #ifndef HAVE_INT128
-  if (isa->extension_enabled('V')) {
+  if (isa->has_any_vector()) {
     fprintf(stderr, "V extension is not supported on platforms without __int128 type\n");
     abort();
   }
@@ -55,7 +55,10 @@ processor_t::processor_t(const isa_parser_t *isa, const cfg_t *cfg,
   }
 #endif
 
-  parse_varch_string(cfg->varch);
+  VU.VLEN = isa->get_vlen();
+  VU.ELEN = isa->get_elen();
+  VU.vlenb = isa->get_vlen() / 8;
+  VU.vstart_alu = 0;
 
   register_base_instructions();
   mmu = new mmu_t(sim, cfg->endianness, this);
@@ -102,11 +105,6 @@ static void bad_option_string(const char *option, const char *value,
   abort();
 }
 
-static void bad_varch_string(const char* varch, const char *msg)
-{
-  bad_option_string("--varch", varch, msg);
-}
-
 static std::string get_string_token(std::string str, const char delimiter, size_t& pos)
 {
   size_t _pos = pos;
@@ -114,17 +112,6 @@ static std::string get_string_token(std::string str, const char delimiter, size_
   return str.substr(_pos, pos - _pos);
 }
 
-static int get_int_token(std::string str, const char delimiter, size_t& pos)
-{
-  size_t _pos = pos;
-  while (pos < str.length() && str[pos] != delimiter) {
-    if (!isdigit(str[pos]))
-      bad_varch_string(str.c_str(), "Unsupported value"); // An integer is expected
-    ++pos;
-  }
-  return (pos == _pos) ? 0 : stoi(str.substr(_pos, pos - _pos));
-}
-
 static bool check_pow2(int val)
 {
   return ((val & (val - 1))) == 0;
@@ -138,51 +125,6 @@ static std::string strtolower(const char* str)
   return res;
 }
 
-void processor_t::parse_varch_string(const char* s)
-{
-  std::string str = strtolower(s);
-  size_t pos = 0;
-  size_t len = str.length();
-  int vlen = 0;
-  int elen = 0;
-  int vstart_alu = 0;
-
-  while (pos < len) {
-    std::string attr = get_string_token(str, ':', pos);
-
-    ++pos;
-
-    if (attr == "vlen")
-      vlen = get_int_token(str, ',', pos);
-    else if (attr == "elen")
-      elen = get_int_token(str, ',', pos);
-    else if (attr == "vstartalu")
-      vstart_alu = get_int_token(str, ',', pos);
-    else
-      bad_varch_string(s, "Unsupported token");
-
-    ++pos;
-  }
-
-  // The integer should be the power of 2
-  if (!check_pow2(vlen) || !check_pow2(elen)) {
-    bad_varch_string(s, "The integer value should be the power of 2");
-  }
-
-  /* Vector spec requirements. */
-  if (vlen < elen)
-    bad_varch_string(s, "vlen must be >= elen");
-
-  /* spike requirements. */
-  if (vlen > 4096)
-    bad_varch_string(s, "vlen must be <= 4096");
-
-  VU.VLEN = vlen;
-  VU.ELEN = elen;
-  VU.vlenb = vlen / 8;
-  VU.vstart_alu = vstart_alu;
-}
-
 static int xlen_to_uxl(int xlen)
 {
   if (xlen == 32)

riscv/processor.h

@@ -273,6 +273,9 @@ public:
   bool any_custom_extensions() const {
     return !custom_extensions.empty();
   }
+  bool any_vector_extensions() const {
+    return VU.VLEN > 0;
+  }
   bool extension_enabled(unsigned char ext) const {
     return extension_enabled(isa_extension_t(ext));
   }
@@ -407,7 +410,6 @@ private:
   friend class plic_t;
   friend class extension_t;
 
-  void parse_varch_string(const char*);
   void parse_priv_string(const char*);
   void build_opcode_map();
   void register_base_instructions();

riscv/riscv.ac

@@ -20,12 +20,6 @@ AC_ARG_WITH(priv,
   AC_DEFINE_UNQUOTED([DEFAULT_PRIV], "$withval", [Default value for --priv switch]),
   AC_DEFINE_UNQUOTED([DEFAULT_PRIV], "MSU", [Default value for --priv switch]))
 
-AC_ARG_WITH(varch,
-	[AS_HELP_STRING([--with-varch=vlen:128,elen:64],
-		[Sets the default vector config])],
-  AC_DEFINE_UNQUOTED([DEFAULT_VARCH], "$withval", [Default value for --varch switch]),
-  AC_DEFINE([DEFAULT_VARCH], ["vlen:128,elen:64"], [Default value for --varch switch]))
-
 AC_ARG_WITH(target,
 	[AS_HELP_STRING([--with-target=riscv64-unknown-elf],
 		[Sets the default target config])],

riscv/v_ext_macros.h

@@ -1449,8 +1449,8 @@ reg_t index[P.VU.vlmax]; \
 #define VI_VFP_COMMON \
   require_fp; \
   require((P.VU.vsew == e16 && p->extension_enabled(EXT_ZVFH)) || \
-          (P.VU.vsew == e32 && p->extension_enabled('F')) || \
-          (P.VU.vsew == e64 && p->extension_enabled('D'))); \
+          (P.VU.vsew == e32 && p->get_isa().get_zvf()) || \
+          (P.VU.vsew == e64 && p->get_isa().get_zvd())); \
   require_vector(true); \
   require(STATE.frm->read() < 0x5); \
   reg_t UNUSED vl = P.VU.vl->read(); \
@@ -1662,8 +1662,8 @@ reg_t index[P.VU.vlmax]; \
 #define VI_VFP_VV_LOOP_WIDE_REDUCTION(BODY16, BODY32) \
   VI_CHECK_REDUCTION(true) \
   VI_VFP_COMMON \
-  require((P.VU.vsew == e16 && p->extension_enabled('F')) || \
-          (P.VU.vsew == e32 && p->extension_enabled('D'))); \
+  require((P.VU.vsew == e16 && p->get_isa().get_zvf()) || \
+          (P.VU.vsew == e32 && p->get_isa().get_zvd())); \
   bool is_active = false; \
   switch (P.VU.vsew) { \
     case e16: { \
@@ -1948,17 +1948,17 @@ reg_t index[P.VU.vlmax]; \
   switch (P.VU.vsew) { \
     case e16: \
       { VI_VFP_CVT_LOOP(CVT_INT_TO_FP_PARAMS(16, 16, sign), \
-        { p->extension_enabled(EXT_ZVFH); }, \
+        { require(p->extension_enabled(EXT_ZVFH)); },   \
         BODY16); } \
       break; \
     case e32: \
       { VI_VFP_CVT_LOOP(CVT_INT_TO_FP_PARAMS(32, 32, sign), \
-        { p->extension_enabled('F'); }, \
+        { require(p->get_isa().get_zvf()); },  \
         BODY32); } \
       break; \
     case e64: \
       { VI_VFP_CVT_LOOP(CVT_INT_TO_FP_PARAMS(64, 64, sign), \
-        { p->extension_enabled('D'); }, \
+        { require(p->get_isa().get_zvd()); },  \
         BODY64); } \
       break; \
     default: \
@@ -1972,17 +1972,17 @@ reg_t index[P.VU.vlmax]; \
   switch (P.VU.vsew) { \
     case e16: \
       { VI_VFP_CVT_LOOP(CVT_FP_TO_INT_PARAMS(16, 16, sign), \
-        { p->extension_enabled(EXT_ZVFH); }, \
+        { require(p->extension_enabled(EXT_ZVFH)); },   \
         BODY16); } \
       break; \
     case e32: \
       { VI_VFP_CVT_LOOP(CVT_FP_TO_INT_PARAMS(32, 32, sign), \
-        { p->extension_enabled('F'); }, \
+        { require(p->get_isa().get_zvf()); },  \
         BODY32); } \
       break; \
     case e64: \
       { VI_VFP_CVT_LOOP(CVT_FP_TO_INT_PARAMS(64, 64, sign), \
-        { p->extension_enabled('D'); }, \
+        { require(p->get_isa().get_zvd()); },  \
         BODY64); } \
       break; \
     default: \

spike_main/spike.cc

@@ -45,7 +45,6 @@ static void help(int exit_code = 1)
   fprintf(stderr, "  --pmpregions=<n>      Number of PMP regions [default 16]\n");
   fprintf(stderr, "  --pmpgranularity=<n>  PMP Granularity in bytes [default 4]\n");
   fprintf(stderr, "  --priv=<m|mu|msu>     RISC-V privilege modes supported [default %s]\n", DEFAULT_PRIV);
-  fprintf(stderr, "  --varch=<name>        RISC-V Vector uArch string [default %s]\n", DEFAULT_VARCH);
   fprintf(stderr, "  --pc=<address>        Override ELF entry point\n");
   fprintf(stderr, "  --hartids=<a,b,...>   Explicitly specify hartids, default is 0,1,...\n");
   fprintf(stderr, "  --ic=<S>:<W>:<B>      Instantiate a cache model with S sets,\n");
@@ -404,7 +403,6 @@ int main(int argc, char** argv)
   parser.option(0, "pmpregions", 1, [&](const char* s){cfg.pmpregions = atoul_safe(s);});
   parser.option(0, "pmpgranularity", 1, [&](const char* s){cfg.pmpgranularity = atoul_safe(s);});
   parser.option(0, "priv", 1, [&](const char* s){cfg.priv = s;});
-  parser.option(0, "varch", 1, [&](const char* s){cfg.varch = s;});
   parser.option(0, "device", 1, device_parser);
   parser.option(0, "extension", 1, [&](const char* s){extensions.push_back(find_extension(s));});
   parser.option(0, "dump-dts", 0, [&](const char UNUSED *s){dump_dts = true;});