Merge pull request #1177 from jordancarlin/wsim_flags

wsim flag fixes and general cleanup

.gitignore

@@ -12,6 +12,7 @@
 *.map
 *.elf*
 *.list
+*.memfile
 
 # General directories to ignore
 .vscode/

bin/wsim

@@ -13,6 +13,7 @@
 
 import argparse
 import os
+import sys
 
 # Global variable
 WALLY = os.environ.get('WALLY')
@@ -31,25 +32,23 @@ def parseArgs():
     parser.add_argument("--params", "-p", help="Optional top-level parameter overrides of the form param=value", default="")
     parser.add_argument("--vcd", "-v", help="Generate testbench.vcd", action="store_true")
     parser.add_argument("--lockstep", "-l", help="Run ImperasDV lock, step, and compare.", action="store_true")
-    parser.add_argument("--locksteplog", "-b", help="Retired instruction number to be begin logging.", default=0)
     parser.add_argument("--lockstepverbose", "-lv", help="Run ImperasDV lock, step, and compare with tracing enabled", action="store_true")
-    parser.add_argument("--covlog", "-d", help="Log coverage after n instructions.", default=0)
     parser.add_argument("--rvvi", "-r", help="Simulate rvvi hardware interface and ethernet.", action="store_true")
     return parser.parse_args()
 
 def validateArgs(args):
     if not args.testsuite and not args.elf:
         print("Error: Missing test suite or ELF file")
-        exit(1)
+        sys.exit(1)
-    if args.lockstep and not args.testsuite.endswith('.elf') and args.testsuite != "buildroot" :
+    if any([args.lockstep, args.lockstepverbose, args.fcov]) and not (args.testsuite.endswith('.elf') or args.elf) and args.testsuite != "buildroot":
-        print(f"Invalid Options. Cannot run a testsuite, {args.testsuite} with lockstep. Must run a single elf or buildroot.")
+        print(f"Invalid Options. Cannot run a testsuite, {args.testsuite} with lockstep or fcov. Must run a single elf or buildroot.")
-        exit(1)
+        sys.exit(1)
-    elif (args.gui or args.ccov or args.fcov or args.lockstep or args.lockstepverbose) and args.sim not in ["questa", "vcs"]:
+    elif any([args.gui, args.ccov, args.fcov, args.lockstep, args.lockstepverbose]) and args.sim not in ["questa", "vcs"]:
         print("Option only supported for Questa and VCS")
-        exit(1)
+        sys.exit(1)
     elif (args.tb == "testbench_fp" and args.sim != "questa"):
         print("Error: testbench_fp presently only supported by Questa, not VCS or Verilator, because of a touchy testbench")
-        exit(1)
+        sys.exit(1)
 
 def elfFileCheck(args):
     ElfFile = ""
@@ -57,124 +56,114 @@ def elfFileCheck(args):
         ElfFile = f"+ElfFile={os.path.abspath(args.elf)}"
     elif args.elf != "":
         print(f"ELF file not found: {args.elf}")
-        exit(1)
+        sys.exit(1)
     elif args.testsuite.endswith('.elf'): # No --elf argument; check if testsuite has a .elf extension and use that instead
         if os.path.isfile(args.testsuite):
             ElfFile = f"+ElfFile={os.path.abspath(args.testsuite)}"
             # extract the elf name from the path to be the test suite
             fields = args.testsuite.rsplit('/', 3)
             # if the name is just ref.elf in a deep path (riscv-arch-test/wally-riscv-arch-test), then use the directory name as the test suite to make it unique; otherwise work directory will have duplicates.
-            if (len(fields) > 3):
+            if len(fields) > 3:
-                if (fields[2] == "ref"):
+                if fields[2] == "ref":
                     args.testsuite = f"{fields[1]}_{fields[3]}"
                 else:
                     args.testsuite = f"{fields[2]}_{fields[3]}"
-            elif ('/' in args.testsuite):
+            elif '/' in args.testsuite:
                 args.testsuite=args.testsuite.rsplit('/', 1)[1] # strip off path if present
         else:
             print(f"ELF file not found: {args.testsuite}")
-            exit(1)
+            sys.exit(1)
     return ElfFile
 
 def prepSim(args, ElfFile):
-    flags = ""
+    prefix = ""
+    paramsList = []
+    argsList = []
+    flagsList = []
     if args.vcd:
-        args.args += " -DMAKEVCD=1"
+        paramsList.append("MAKE_VCD=1")
     if args.rvvi:
-        args.params += " RVVI_SYNTH_SUPPORTED=1 "
+        paramsList.append("RVVI_SYNTH_SUPPORTED=1")
     if args.tb == "testbench_fp":
-        args.params += f' TEST="{args.testsuite}" '
+        paramsList.append(f'TEST="{args.testsuite}"')
-    if ElfFile != "":
+    if ElfFile:
-        args.args += f" {ElfFile}"
+        argsList.append(f"{ElfFile}")
+    if args.gui and args.tb == "testbench":
+        paramsList.append("DEBUG=1")
     if args.ccov:
-        flags += " --ccov"
+        flagsList.append("--ccov")
     if args.fcov:
-        flags += " --fcov"
+        flagsList.append("--fcov")
-    prefix, suffix = lockstepSetup(args)
+    if args.gui:
-    flags += suffix
+        flagsList.append("--gui")
+    if args.lockstep or args.lockstepverbose:
+        flagsList.append("--lockstep")
+    if args.lockstep or args.lockstepverbose or args.fcov:
+        prefix = lockstepSetup(args)
+    # Combine into a single string
+    args.args += " ".join(argsList)
+    args.params += " ".join(paramsList)
+    flags = " ".join(flagsList)
     return flags, prefix
 
 def lockstepSetup(args):
-    prefix = ""
+    imperasicVerbosePath = os.path.join(WALLY, "sim", "imperas-verbose.ic")
-    suffix = ""
-    ImperasPlusArgs = ""
-
-    if(int(args.locksteplog) >= 1): EnableLog = 1
-    else: EnableLog = 0
-    if (args.lockstep or args.lockstepverbose or args.fcov):
     imperasicPath = os.path.join(WALLY, "config", args.config, "imperas.ic")
     if not os.path.isfile(imperasicPath): # If config is a derivative, look for imperas.ic in derivative configs
         imperasicPath = os.path.join(WALLY, "config", "deriv", args.config, "imperas.ic")
         if not os.path.isfile(imperasicPath):
             print("Error: imperas.ic not found")
-                exit(1)
+            sys.exit(1)
-        prefix += f"IMPERAS_TOOLS={imperasicPath}"
+    prefix = f"IMPERAS_TOOLS={imperasicPath}{f':{imperasicVerbosePath}' if args.lockstepverbose else ''}"
+    return prefix
 
-    if (args.lockstep or args.lockstepverbose):
+def createDirs(sim):
-        if(args.locksteplog != 0): ImperasPlusArgs = f" +IDV_TRACE2LOG={EnableLog} +IDV_TRACE2LOG_AFTER={args.locksteplog}"
-        if(args.fcov):
-            CovEnableStr = "1" if int(args.covlog) > 0  else "0"
-            if(args.covlog >= 1): EnableLog = 1
-            else: EnableLog = 0
-            ImperasPlusArgs = f" +IDV_TRACE2COV={EnableLog} +TRACE2LOG_AFTER={args.covlog} +TRACE2COV_ENABLE={CovEnableStr}"
-        else:
-            suffix = "--lockstep"
-        if(args.lockstepverbose):
-            prefix += f":{WALLY}/sim/imperas-verbose.ic"
-    args.args += ImperasPlusArgs
-    return prefix, suffix
-
-def createDirs(args):
     for d in ["logs", "wkdir", "cov", "ucdb", "fcov", "fcov_ucdb"]:
-        os.makedirs(os.path.join(WALLY, "sim", args.sim, d), exist_ok=True)
+        os.makedirs(os.path.join(WALLY, "sim", sim, d), exist_ok=True)
 
 def runSim(args, flags, prefix):
-    if (args.sim == "questa"):
+    if args.sim == "questa":
         runQuesta(args, flags, prefix)
-    elif (args.sim == "verilator"):
+    elif args.sim == "verilator":
-        runVerilator(args, flags, prefix)
+        runVerilator(args)
-    elif (args.sim == "vcs"):
+    elif args.sim == "vcs":
         runVCS(args, flags, prefix)
 
 def runQuesta(args, flags, prefix):
     # Force Questa to use 64-bit mode, sometimes it defaults to 32-bit even on 64-bit machines
     prefix = "MTI_VCO_MODE=64 " + prefix
-    if (args.gui) and (args.tb == "testbench"):
+    if args.args != "":
-        args.params += "DEBUG=1"
+        args.args = fr'--args \"{args.args}\"'
-    if (args.args != ""):
+    if args.params != "":
-        args.args = f' --args \\"{args.args}\\"'
+        args.params = fr'--params \"{args.params}\"'
-    if (args.params != ""):
-        args.params = f' --params \\"{args.params}\\"'
     # Questa cannot accept more than 9 arguments.  fcov implies lockstep
     cmd = f"do wally.do {args.config} {args.testsuite} {args.tb} {args.args} {args.params} {flags}"
-    if (args.gui):  # launch Questa with GUI; add +acc to keep variables accessible
+    cmd = f'cd $WALLY/sim/questa; {prefix} vsim {"-c" if not args.gui else ""} -do "{cmd}"'
-        cmd = f'cd $WALLY/sim/questa; {prefix} vsim -do "{cmd}  +acc"'
-    else: # launch Questa in batch mode
-        cmd = f'cd $WALLY/sim/questa; {prefix} vsim -c -do "{cmd}"'
     print(f"Running Questa with command: {cmd}")
     os.system(cmd)
 
-def runVerilator(args, flags, prefix):
+def runVerilator(args):
     print(f"Running Verilator on {args.config} {args.testsuite}")
     os.system(f'make -C {WALLY}/sim/verilator WALLYCONF={args.config} TEST={args.testsuite} TESTBENCH={args.tb} PLUS_ARGS="{args.args}" PARAM_ARGS="{args.params}"')
 
 def runVCS(args, flags, prefix):
     print(f"Running VCS on {args.config} {args.testsuite}")
-    # if (args.gui):
+    if args.args != "":
-    #     flags += " --gui"
-    if (args.args != ""):
         args.args = f'--args "{args.args}"'
-    if (args.params != ""):
+    if args.params != "":
         args.params = f'--params "{args.params}"'
     cmd = f"cd $WALLY/sim/vcs; {prefix} ./run_vcs {args.config} {args.testsuite} --tb {args.tb} {args.args} {args.params} {flags}"
     print(cmd)
     os.system(cmd)
 
-if __name__ == "__main__":
+def main(args):
-    args = parseArgs()
     validateArgs(args)
     print(f"Config={args.config} tests={args.testsuite} sim={args.sim} gui={args.gui} args='{args.args}' params='{args.params}'")
     ElfFile = elfFileCheck(args)
     flags, prefix = prepSim(args, ElfFile)
-    createDirs(args)
+    createDirs(args.sim)
-    exit(runSim(args, flags, prefix))
+    sys.exit(runSim(args, flags, prefix))
+
+if __name__ == "__main__":
+    args = parseArgs()
+    main(args)

sim/questa/wally.do

@@ -8,7 +8,7 @@
 #
 # Takes 1:10 to run RV64IC tests using gui
 
-# Usage: do wally.do <config> <testcases> <testbench> [--ccov] [--fcov] [+acc] [--args "any number of +value"] [--params "any number of VAR=VAL parameter overrides"]
+# Usage: do wally.do <config> <testcases> <testbench> [--ccov] [--fcov] [--gui] [--args "any number of +value"] [--params "any number of VAR=VAL parameter overrides"]
 # Example: do wally.do rv64gc arch64i testbench
 
 # Use this wally.do file to run this example.
@@ -40,7 +40,6 @@ set TESTSUITE ${2}
 set TESTBENCH ${3}
 set WKDIR wkdir/${CFG}_${TESTSUITE}
 set WALLY $::env(WALLY)
-set IMPERAS_HOME $::env(IMPERAS_HOME)
 set CONFIG ${WALLY}/config
 set SRC ${WALLY}/src
 set TB ${WALLY}/testbench
@@ -91,7 +90,7 @@ echo "number of args = $argc"
 echo "lst = $lst"
 
 # if +acc found set flag and remove from list
-if {[lcheck lst "+acc"]} {
+if {[lcheck lst "--gui"]} {
     set GUI 1
     set accFlag "+acc"
 }
@@ -118,6 +117,7 @@ if {[lcheck lst "--fcov"]} {
 
 # if --lockstep or --fcov found set flag and remove from list
 if {[lcheck lst "--lockstep"] || $FunctCoverage == 1} {
+    set IMPERAS_HOME $::env(IMPERAS_HOME)
     set lockstep 1
     set lockstepvlog "+define+USE_IMPERAS_DV \
                       +incdir+${IMPERAS_HOME}/ImpPublic/include/host \

sim/vcs/run_vcs

@@ -24,8 +24,8 @@ parser = argparse.ArgumentParser()
 parser.add_argument("config", help="Configuration file")
 parser.add_argument("testsuite", help="Test suite (or none, when running a single ELF file) ")
 parser.add_argument("--tb", "-t", help="Testbench", choices=["testbench", "testbench_fp"], default="testbench")
-parser.add_argument("--coverage", "-c", help="Code & Functional Coverage", action="store_true")
+parser.add_argument("--ccov", "-c", help="Code Coverage", action="store_true")
-parser.add_argument("--fcov", "-f", help="Code & Functional Coverage", action="store_true")
+parser.add_argument("--fcov", "-f", help="Functional Coverage", action="store_true")
 parser.add_argument("--args", "-a", help="Optional arguments passed to simulator via $value$plusargs", default="")
 parser.add_argument("--params", "-p", help="Optional top-level parameter overrides of the form param=value", default="")
 parser.add_argument("--lockstep", "-l", help="Run ImperasDV lock, step, and compare.", action="store_true")
@@ -65,7 +65,7 @@ else:
     LOCKSTEP_SIMV = ""
 
 # coverage mode
-if (args.coverage):
+if (args.ccov):
     COV_OPTIONS = "-cm line+cond+branch+fsm+tgl -cm_log " + wkdir + "/coverage.log -cm_dir " + wkdir + "/coverage"
 else:
     COV_OPTIONS = ""
@@ -93,6 +93,6 @@ SIMV_CMD= wkdir + "/" + OUTPUT + " +TEST=" + args.testsuite + " " + args.args +
 print("Executing: " + str(VCS) )
 subprocess.run(VCS, shell=True)
 subprocess.run(SIMV_CMD, shell=True)
-if (args.coverage):
+if (args.ccov):
     COV_RUN = "urg -dir " + wkdir + "/coverage.vdb -format text -report IndividualCovReport/" + args.config + "_" + args.testsuite
     subprocess.run(COV_RUN, shell=True)

sim/verilator/Makefile

@@ -21,8 +21,6 @@ TESTBENCH?=testbench
 
 # constants
 # assume WALLY variable is correctly configured in the shell environment
-WORKING_DIR=${WALLY}/sim/verilator
-TARGET=$(WORKING_DIR)/target
 # INCLUDE_PATH are pathes that Verilator should search for files it needs
 INCLUDE_PATH="-I${WALLY}/config/shared" "-I${WALLY}/config/$(WALLYCONF)" "-I${WALLY}/config/deriv/$(WALLYCONF)"
 # SOURCES are source files
@@ -30,6 +28,8 @@ SOURCES=${WALLY}/src/cvw.sv ${WALLY}/testbench/${TESTBENCH}.sv ${WALLY}/testbenc
 # DEPENDENCIES are configuration files and source files, which leads to recompilation of executables
 DEPENDENCIES=${WALLY}/config/shared/*.vh $(SOURCES)
 
+WORKDIR = $(VERILATOR_DIR)/wkdir/$(WALLYCONF)_$(TEST)
+
 # regular testbench requires a wrapper defining getenvval
 ifeq ($(TESTBENCH), testbench)
 	WRAPPER=${WALLY}/sim/verilator/wrapper.c
@@ -41,9 +41,9 @@ endif
 
 default: run
 
-run: wkdir/$(WALLYCONF)_$(TEST)/V${TESTBENCH}
+run: $(WORKDIR)/V${TESTBENCH}
 	mkdir -p $(VERILATOR_DIR)/logs
-	wkdir/$(WALLYCONF)_$(TEST)/V${TESTBENCH} ${ARGTEST} $(PLUS_ARGS)
+	$(WORKDIR)/V${TESTBENCH} ${ARGTEST} $(PLUS_ARGS)
 
 profile: obj_dir_profiling/V${TESTBENCH}_$(WALLYCONF)
 	$(VERILATOR_DIR)/obj_dir_profiling/V${TESTBENCH}_$(WALLYCONF) ${ARGTEST}
@@ -54,10 +54,10 @@ profile: obj_dir_profiling/V${TESTBENCH}_$(WALLYCONF)
 	mv gmon_$(WALLYCONF)* $(VERILATOR_DIR)/logs_profiling
 	echo "Please check $(VERILATOR_DIR)/logs_profiling/gmon_$(WALLYCONF)* for logs and output files."
 
-wkdir/$(WALLYCONF)_$(TEST)/V${TESTBENCH}: $(DEPENDENCIES)
+$(WORKDIR)/V${TESTBENCH}: $(DEPENDENCIES)
-	mkdir -p wkdir/$(WALLYCONF)_$(TEST)
+	mkdir -p $(WORKDIR)
 	verilator \
-	--Mdir wkdir/$(WALLYCONF)_$(TEST) -o V${TESTBENCH} \
+	--Mdir $(WORKDIR) -o V${TESTBENCH} \
 	--binary --trace \
 	$(OPT) $(PARAMS) $(NONPROF) \
 	--top-module ${TESTBENCH}  --relative-includes \

testbench/testbench.sv

@@ -44,6 +44,7 @@ module testbench;
   parameter I_CACHE_ADDR_LOGGER=0;
   parameter D_CACHE_ADDR_LOGGER=0;
   parameter RVVI_SYNTH_SUPPORTED=0;
+  parameter MAKE_VCD=0;
 
   `ifdef USE_IMPERAS_DV
     import idvPkg::*;
@@ -230,10 +231,10 @@ module testbench;
       end
       $finish;
     end
-`ifdef MAKEVCD
+    if (MAKE_VCD) begin
       $dumpfile("testbench.vcd");
       $dumpvars;
-`endif
+    end
   end // initial begin
 
   // Model the testbench as an fsm.