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

Artifacts #20

Closed coreyeng closed 6 years ago

coreyeng commented 6 years ago

Reinstated artifacts to allow OrigenSim to move files from wherever to the run_dir for us, without regards to vendor or target.

For my case, artifacts ended up being flash firmware, some default fuse-like values, some basic ROM code, and an analog settings file. The first few are loaded in the testbench during an initial block using readmemh, which had a path relative to the run directory. Previously, these would have to be manually moved and we'd need to know where OrigenSim was planning to run the simulation beforehand. Now, OrigenSim will handle them itself.

The README was updated to show some basic usages. I used Origen::Componentable for this, which even though I found some bugs I'll need to fix back in that, they were easy enough to work around to get a release of OrigenSim out. However, expanding artifacts in the future should be very easy, should we need to.

ginty commented 6 years ago

Looks good, but why root/simulation/_artifacts_ and not root/simulation/artifacts ?

coreyeng commented 6 years ago

I've been using either or ..._ to represent special names when the input could be anything, and that bled over into this. I think it'd be far more reasonable for a user to have an 'artifacts' target than an 'artifacts' target, so I went to the latter just to make the chance of clashing very small.

Also, I think with the underscores it stands out more, further pointing out that its a special-use directory. If someone sees it but doesn't know what it is, I think they'd be less inclined to think its just another snapshot directory.

ginty commented 6 years ago

Not sure I really like this naming convention, I think the need for it here is really indicating that the choice of location is not quite right.

What about vendor/artifacts instead?

Or simulation/<target>/artifacts ?

coreyeng commented 6 years ago

I was trying to keep artifacts as something to persists across targets, so I don't think simulation/\<target>/artifacts is the proper location. Since artifacts are only OrigenSim related, do you think vendor is best for them? I think of them more as 'in addition to the vendor' instead of 'vendor provided', though I guess that depends on the actual artifacts in question. For my use, they are definitely the former.

What about plugin namespaced? simulation/origen_sim/artifacts? Or sim environment name based? environment/\<sim environement name>/artifacts? -> environment/sim/artifacts for most of our stuff. Or application/artifacts or application/origen_sim/artifacts, following closer to how Rails organizes its applications.

I'm also not sure if artifacts is a key directory for any vendors either, so _artifacts_ seemed a safe bet for something to just copy to the run_dir without any side effects. I've only ever used Cadence tools and ModelSim, so my vendor experience is limited.

ginty commented 6 years ago

I think simulation/application/artifacts for simulation artifacts that you want available to all of your targets is a good choice.

In fact implementing it such that you can override default application artifact files by putting a file of the same name in simuation/<target>/artifacts would be good.

That should give you enough control for most situations without even having to touch the config, but you can still fall back to providing explicit file references in the sim config if you need to.

coreyeng commented 6 years ago

Here' s an example of what this looks like. In this example, I've set my artifactdir to "#{Origen.app.root}/simulation/_testbench\" and my artifact_run_dir to ./.

I've got three artifacts at the simulation/_testbench_: defines.v, top.scs, memories/, and I overwrite top.scs, memories/ by placing alternatives at simulation/default/artifacts.

image

Should the override directory change with the updated artifactdir? The override directory being simulation/default/_testbench\ instead?

coreyeng commented 6 years ago

Also tossing in a fix for Ruby 2.3 wherein the heartbeat process outlives the simulation process, even after being closed, by a beat. However, by then the simulation process is dead and the IO streams have been closed, leading to the heartbeat process to error out instead of exit gracefully.

image

ginty commented 6 years ago

Looks nice.

If you specify an artifact dir, is it more intuitive/useful if that is simply added as another level with highest priority? i.e.

coreyeng commented 6 years ago

I agree, since we've added overriding and now have a reserved simulation directory at application, that makes more sense now.