Fix broken verilator permissive runtime arguments

ucb-bar / chipyard

An Agile RISC-V SoC Design Framework with in-order cores, out-of-order cores, accelerators, and more

https://chipyard.readthedocs.io/en/stable/

BSD 3-Clause "New" or "Revised" License

1.63k stars 645 forks source link

Fix broken verilator permissive runtime arguments #563

Closed colinschmidt closed 4 years ago

colinschmidt commented 4 years ago

So we do need permissive and its even slightly different for verilator so its good that its overrideable:

simulator-example-RocketConfig +max-cycles=5000 +verbose -- +permissive +verilator+V +permissive-off output/example.TestHarness.RocketConfig/rv64ui-p-addi
using random seed 1590055841
  Version: Verilator 4.016 2019-06-16
%Error: COMMAND_LINE:0: Exiting due to command line argument (not an error)
Aborting...
Aborted (core dumped)

versus

simulator-example-RocketConfig +max-cycles=5000 +verbose +verilator+V output/example.TestHarness.RocketConfig/rv64ui-p-addi
simulator-example-RocketConfig: invalid plus-arg (Verilog or HTIF) "+verilator+V"

_Originally posted by @colinschmidt in https://github.com/ucb-bar/chipyard/pull/562/review_comment/create_

colinschmidt commented 4 years ago

I'm not gonna run git blame but I believe this is a regression.

jwright6323 commented 4 years ago

I'll be that guy and point the finger at @jerryz123 https://github.com/ucb-bar/chipyard/commit/340ed90652c58ec3228706f49463cb12920f1307 :)

jwright6323 commented 4 years ago

Why did it core dump if it wasn't an error?

colinschmidt commented 4 years ago

Thats just the flag I passed to show it was working. If you give it one of the random init flags it just works silently.

colinschmidt commented 4 years ago

Its a version flag

jwright6323 commented 4 years ago

Yeah, I get that, just don't get why the version flag causes a core dump. Not really a question directed at you, just seems odd.

jerryz123 commented 4 years ago

I believe I made that change to match existing project-template behavior, as the project-template verilator Makefile (and the Rocketchip verilator Makefile, too), do not set the permissive flags.

I'm not sure what the right thing to do here is, but it seems to stem from the fact that we really have two sets of arguments, 1 set of arguments for HTIF, and another for the simulator. Combining them into one set of args is convenient, but then we have to deal with these kinds of issues.

colinschmidt commented 4 years ago

I think we should add this feature since we already have the infrastructure to support it and I did the work of finding what you would need to set them to.

jwright6323 commented 4 years ago

@colinschmidt so I'm interpreting this to mean that the simulation/emulator.cc doesn't barf on +permissive when you use a non-terminal +verilator+... option? Just asking because most other "Custom" plusargs need to be explicitly added to verilator's C code somehow (hence https://github.com/chipsalliance/rocket-chip/pull/2453). Is +permissive a verilog thing or an htif thing?

colinschmidt commented 4 years ago

Yes I can run verilator simulations to completion with this line:

simulator-example-RocketConfig +max-cycles=5000 +verbose -- +permissive +verilator+rand+reset+0 +permissive-off output/example.TestHarness.RocketConfig/rv64ui-p-addi

colinschmidt commented 4 years ago

+permissive is a htif thing.

jwright6323 commented 4 years ago

cool... this seems like a really easy fix, no?

colinschmidt commented 4 years ago

yeah I think its a few lines of PR, but I won't get to it until the weekend at the earliest