Add Vivado XSim as a simulator #2363
Conversation
| @@ -329,7 +329,7 @@ fn emit_component<F: io::Write>( | |||
| if let Some(check) = | |||
| emit_guard_disjoint_check(dst, asgns, &pool, true) | |||
| { | |||
| writeln!(f, "always_comb begin")?; | |||
| writeln!(f, "always_ff @(posedge clk) begin")?; | |||
There was a problem hiding this comment.
Can you explain this change? On the face of it, it is turning combinational logic into sequential logic and I'm not sure why I would expect this to be correct?
There was a problem hiding this comment.
Yeah this has to do with the difference between cycle based simulators like verilator (and icarus) and time step based simulators (like xsim and modelsim). Cycle based simulators evaluate all assignments in a cycle first and then all asserts. Time step based simulators progress through assignments in continuous small time steps, meaning all values in the cycle may not be evaluated before the asserts are evaluated. At some point during the cycle while evaluations are occurring it might be the case that one one-hot value has updated but another hasn't (essentially like a propagation delay) which results in multiple values being high at the same time. Basically checking the asserts combinationally is too strict for a timing based simulator to be able to handle properly. This version should also behave nearly identically on cycle based simulators because the entire cycle is evaluated before asserts are.
| [-help|-h] | ||
|
|
||
| Description: xxxxxxxxxxxxxxxxxxx. | ||
| xxxxxxxxxxxxxxxxxxx. |
There was a problem hiding this comment.
We should probably provide a helpful description for this script here.
| @@ -20,7 +20,7 @@ fn xilinx_setup(e) { | |||
| ); | |||
| e.arg("pool", "console"); // Lets Ninja stream the tool output "live." | |||
|
|
|||
| // Compile an `.xo` file to an `.xclbin` file, which is where the actual EDA work occurs. | |||
| // Compile an `.xo` ile to an `.xclbin` file, which is where the actual EDA work occurs. | |||
|
Thanks for getting us started on this! Having support for XSim is definitely a big win and we should get this merged. Couple of questions for the code but broadly, could you explain why the changes are needed? |
There was a problem hiding this comment.
LGTM! (My comments are just nitpicking on code that was commented out)
I doubt that you'd need to use the custom testbench op right now since we haven't migrated to the custom testbench yet (so the verilog_refmem state is mostly/only used for the FIRRTL backend). But I think it's great to have around since we hope to make that migration sometime soon!!
| exprs: vec
I'll address the comments in more detail soon, and I gave a specific example in my comment, but these changes are largely needed because of timing vs cycle based simulators. Almost all commercial simulators are timing based so we definitely want to be able to work correctly there. Timing simulators are also typically more strict so changes to get things to work for timing simulators should not impact behavior for cycle-based simulators. |
|
The other changes are because 1. xsim has a bug with string concatenation until really recent versions that results in garbage results and 2. the way we were doing verilog macro defines in verilator and iverilog did not match how most commercial simulators work so xsim wasn't handling the macros correctly. Basically all of these changes have to do with differences between the behavior of commercial and open-source simulators which is a bit annoying but it should work for both now. |
|
I'm pretty sure this breaks some fud1 behavior but I haven't looked into it much. Not saying we should do it now but I feel like fud2 is starting to surpass the functionality of fud1 and having both in the codebase just makes improvements harder. |
Adds vivado xsim as a choice of simulator to fud2 because I want to use some encrypted IPs that can't be simulated by Verilator. XSim is slower than verilator by a lot so little reason for others to use it unless they want to use Xilinx floating point IP. It also behaves differently on some verilog constructs, so I had to make some fixes there (these fixes should also help calyx generated verilog work better on all time based (as opposed to cycle based like verilator) simulators).
Not really sure how to update the fud2 test cases but I won't do that until people agree with the other changes anyways.