Origen-SDK / origen_sim

Plugin to enable Origen patterns to be run in a dynamic Verilog simulation
MIT License
1 stars 4 forks source link

Generic socket2 - Merged Generic Socket with master and added some features. #11

Closed coreyeng closed 6 years ago

coreyeng commented 6 years ago

This PR adds a generic toolchain/vendor option, in the same way the #10 did, but uses the options generic_run_cmd now, since post_run_start_block isn't really correct anymore.

E.g.:

OrigenSim.generic(startup_timeout: 900) do |sim|
  sim.testbench_top 'na_origen'
  sim.generic_run_cmd do |s|
    "ncsim na_origen -loadpli origen.so:bootstrap +socket+#{s.socket_id}"
  end
end

All the other changes still exist as before.

This command will be run in the same way as a supported vendor's would, i.e., using #9 's enhancements.

A few additions that I made:

I'm not sure what to do with post/pre_run_start_block. I don't think these are relevant anymore, but not sure if it would be good to keep since they are already there. Thoughts?

coreyeng commented 6 years ago

Added a slight differentiator to STDERR and STDOUT fails. I think it looks pretty out of the way and shouldn't be confused with raw output.

image

image

coreyeng commented 6 years ago

Then how this looks at the end:

image

coreyeng commented 6 years ago

Thanks! I even proofread these and still had mistakes... I'll fix the Readme.

What do you think about just removing the pre/post_run_start_block options? I'm not sure what use they would have now, since those are still run in the parent process, not the testbench process.

I've kinda gone the way of over-rejecting and being quick to fail the simulation. Since the condition strings are different for stderr and stdout, I thought we should have two separate exception strings and maintain them independently. My thought is that if I originally only removed some string from stdout, then it all of a sudden started appearing in stderr, I'd want to know about it. Not sure if this is easier than just maintaining a base array and concatenating it with both.

What about a middle ground of error_exception_strings that apply to both, then having stdout_exception_strings and stderr_exception_strings. I also think we should add error, Error, *E to the list of default error strings as well, but was planning to do that later.

Maybe too much detail, but I've gone the way of over-rejecting since I don't see our design taking over the snapshot generation here anytime soon. Having to at least look at a bunch of Verilog errors forces us to understand what we're skipping and what it means, especially since I'm just a makeshift verification engineer for this and its real easy to just look a block of text, get a pass result, and not look at it again. Examples of things that I could have fixed in the past with this were:

Not much, but enough to where a forced check of 'what does this error actually mean' would have, I think, been helpful. Then of course they can always just disable everything if they want.

ginty commented 6 years ago

Agree with losing the pre/post run start blocks.

I don't have a strong opinion on the error handling, I think your experience is better and if you prefer the current way I'm fine with that.