edalize icon indicating copy to clipboard operation
edalize copied to clipboard

Add cocotb without makefiles

Open gchqdev1729 opened this issue 4 years ago • 7 comments

See issue https://github.com/olofk/edalize/issues/123 for background.

Following the discussion on my previous PR https://github.com/olofk/edalize/pull/138, I've had a go at the alternative method of calling cocotb from edalize. It still has one or two things to resolve, so I'm putting up the idea as a draft for comments. Generally, I think it's much better, fits with the way cocotb is going and uses the power of edalize.

The idea

This is a bit different from other Edalize backends because rather than running an EDA tool itself, it sets up a few arguments and then delegates the rest of the work to another backend. Those with deeper knowledge of Python or Edalize might spot some problems with my approach that I've missed. (@olofk ?)

The Cocotb backend does not inherit from Edatool. This allows it to instantiate another backend and delegate methods to that using a getattr override. Before it instantiates the other backend, it modifies the edam configuration to add in the appropriate cocotb arguments and set it up as though the second backend had been called directly.

A limitation - passing tool_options for second backend

One notable thing that's missing is a way to pass options to the second backend. I have considered doing that by adding a tool_options section to the cocotb tool configuration in the FuseSoC core file. Something like this:

tools:
      cocotb:
        simulator: ghdl
        tool_options:
          run_options: --some-run-options
        module: test_adder
        toplevel_lang: vhdl

I had planned to copy that tool_options section to the appropriate place when I modify the edam. You can see I've started to implement that in the code. (https://github.com/gchqdev1729/edalize/blob/620f293a2908bd7f6c6c818cde2d0b2656a19b0a/edalize/cocotb.py#L82)

However, it doesn't work because by the time Edalize receives edam, those options have been stripped out. I'm sure there's a way to make this work, but I haven't got it working yet. If this PR looks like a good approach generally, I'll put more effort into fixing this.

CocotbConfig

I've put all the knowledge that I think belongs in the Cocotb codebase within the class CocotbConfig. I think that could be replaced with an API call or call to cocotb-config. The way its implemented just now means that cocotb has to be installed to run the tests, which is not ideal.

Incidental edits

I had to modify the Icarus backend slightly to implement options to vpp.

I had to add the standard to the GHDL run command to make that work with version 0.36. I have not dug into whether that needs to be added as a patch to Edalize anyway.

These changes meant I have touched a fair number of test files in a minor way.

Testing my work

I've provided a pytest module, which bases itself on the icarus backend test.

I've updated my FuseSoC core files (https://github.com/gchqdev1729/fusesoc-cores). The cocotb example, mean, does not work at present - I have not implemented support for any mixed-language capable simulators. The others work though.

gchqdev1729 avatar May 28 '20 13:05 gchqdev1729

The last commit sets the cocotb environment when necessary without making global changes. It makes use of the self.env variable in Edatool. When it gets used, it's necessary to update this with os.environ because some of the backends and their tests make changes subsequent to the backend instantiation.

There are some opportunities for this to go wrong, where the same environment variable is configured in two places, for instance. The tests pass now though, so I think it's safe.

gchqdev1729 avatar May 28 '20 16:05 gchqdev1729

This is very interesting. I think it highlights some of the shortcomings of Edalize as it is now which I hope to rectify with the next major Edalize version that is currently being drafted https://github.com/olofk/edalize/wiki/Edalize-(Slight-return)

Let me know if you need any action from me

olofk avatar Jun 17 '20 10:06 olofk

I did think about cocotb and fusesoc/edalize a bit more, and I think edalize is the wrong place to add support for it. Cocotb is ultimately nothing more than a VPI library. One should be able to already today write a fusesoc .core file with a VPI section which loads the cocotb VPI library.

I can't see hard blockers along the way: write a core file with a VPI section, potentially a couple of them with appropriate tool use flags, and depend on it in your main fusesoc core file where you specify the project source files, parameters, and everything else.

This way we re-use the knowledge in edalize how to call tools, how to pass parameters and defines, etc. without duplicating any of that.

imphil avatar Jun 17 '20 10:06 imphil

This is very interesting. I think it highlights some of the shortcomings of Edalize as it is now which I hope to rectify with the next major Edalize version that is currently being drafted https://github.com/olofk/edalize/wiki/Edalize-(Slight-return)

Let me know if you need any action from me

Thanks for taking a look. The main thing I wanted your input on was whether what I'd written was sensible because it is rather non-standard compared with the other backends. It's really interesting to see your plans, because the passing of edam around between your stages formalises and generalises what I've tried to do.

gchqdev1729 avatar Jun 25 '20 13:06 gchqdev1729

@imphil Thanks for the comment. It's raised a couple of questions.

I did think about cocotb and fusesoc/edalize a bit more, and I think edalize is the wrong place to add support for it. Cocotb is ultimately nothing more than a VPI library. One should be able to already today write a fusesoc .core file with a VPI section which loads the cocotb VPI library.

I can't see hard blockers along the way: write a core file with a VPI section, potentially a couple of them with appropriate tool use flags, and depend on it in your main fusesoc core file where you specify the project source files, parameters, and everything else.

I looked at adding Cocotb as a VPI module as you suggest here and @themperek commented on it here https://github.com/olofk/edalize/pull/162#discussion_r431878632. Maybe I was looking at it the wrong way - I think what you're talking about a different way. Let me make sure I'm understanding your proposal...

Would you have a cocotb core file that solely builds the cocotb VPI module and pulls the code directly from the cocotb repo? The core file could live with cocotb, I suppose, which keeps the knowledge of which files are require in the cocotb codebase. If you've pip-installed cocotb, then pointing FuseSoC at that installation means you can be sure your VPI module matches the Python you have installed.

Then we'd need each top-level core file to add special tool options, for example cocotbvpi_modelsim:cocotbvpi_entry_point for mixed language ModelSim. I don't think there's a way to pass tool options up with VPI modules. (Maybe something that could be added?)

Some simulators don't support VPI in Edalize, for example GHDL, but that's a non-issue really. Easy to fix.

This way we re-use the knowledge in edalize how to call tools, how to pass parameters and defines, etc. without duplicating any of that.

I think I've achieved that with this implementation because most of the work is done by the simulator backend. The Cocotb backend is just a pre-processor, in a way. Definitely wasn't the case with my earlier proposal that I discarded after some helpful input from others.

Assuming I've understood correctly above, there's one aim of my backend that your proposal doesn't cover. I currently build a Python path for hierarchical testbench models (see my example cores and code linked in the description). It's a pattern we currently use a lot. That couldn't work without some special backend code. The other backends don't care about Python code, quite rightly. There might be another way of achieving this, though expressing the dependencies once through FuseSoC is attractive.

gchqdev1729 avatar Jun 25 '20 14:06 gchqdev1729

Would you have a cocotb core file that solely builds the cocotb VPI module

The cocotb VPI libraries (one for each simulator) are built during "pip install", i.e. at cocotb installation time. Like for all other tools, we expect users to perform this step before they use fusesoc/edalize, so it's out of scope. In edalize, we can assume the VPI libraries are compiled and available on the user's hard disk somewhere.

What we need to do in edalize is exactly what the Makefiles in cocotb do at the moment: call a simulator, and pass it

  • a file list
  • parameters/defines, etc.
  • a VPI library
  • potentially other tool-specific options, like the VPI entry point.

None of that is specific to cocotb.

What you correctly highlight is that there are a number of features missing in fusesoc (and by consequence, in edalize) related to

  • VPI support and precompiled libraries
  • passing arguments to tools in a composable and sufficiently generic way
  • Setting and passing environment variables

It would be great if we can come up with a proposal of how we'd like a .core file to look that runs a cocotb simulation, and based on that, a set of issues which are currently preventing that to work. All issues above are pain points for other uses of fusesoc as well, which we will need to address at one point anyways. I opened https://github.com/olofk/fusesoc/issues/417 to start the discussion on that.

I realize that changing direction here is annoying and will move us further away from cocotb support in fusesoc/edalize. To avoid making the better the enemy of good, I'm proposing to do the analysis step of the VPI solution in https://github.com/olofk/fusesoc/issues/417. Let's take a week or two and discuss options there. Then, let's see how much work we have ahead of us for the "VPI solution" as outlined. If it turns out to be too far away, I'm OK with merging this PR as an intermediate step and mark it as "experimental" or so.

imphil avatar Jun 26 '20 14:06 imphil

@imphil - thanks for the comprehensive reply. I understand what you're proposing now and agree it's a better solution. I was starting from the point of not modifying fusesoc, but by doing exactly that we end up with something neater.

The cocotb VPI libraries (one for each simulator) are built during "pip install", i.e. at cocotb installation time. Like for all other tools, we expect users to perform this step before they use fusesoc/edalize, so it's out of scope. In edalize, we can assume the VPI libraries are compiled and available on the user's hard disk somewhere.

Totally agree. It was trying to make use of the compiled libraries with the existing VPI machinery that was troubling me.

I realize that changing direction here is annoying and will move us further away from cocotb support in fusesoc/edalize. To avoid making the better the enemy of good, I'm proposing to do the analysis step of the VPI solution in olofk/fusesoc#417. Let's take a week or two and discuss options there. Then, let's see how much work we have ahead of us for the "VPI solution" as outlined. If it turns out to be too far away, I'm OK with merging this PR as an intermediate step and mark it as "experimental" or so.

Thanks for starting the discussions. I'll head over there in a bit. For what it's worth, I think that a potential direction change after discussion is the right thing to do. I'm pleased that even if my work doesn't make it into the codebase we're on the path towards the end-goal of getting two great projects working together!

It's worth saying that this PR has a few shortcomings that probably require fusesoc mods to fix - in particular passing through tool_options for the simulator backend. So the bar for discarding this approach is not as high as it might be!

gchqdev1729 avatar Jun 29 '20 08:06 gchqdev1729

Lots of things have changed in both Edalize and cocotb since this PR was raised. Most notably, cocotb installation and setup has been streamlined and Edalize has gained the flow API. With that I believe we don't need a separate cocotb backend anymore and instead just need to enable cocotb by setting the correct simulator options. I actually run quite a bit of cootb nowadays and with the Edalize tool API (which is still the most widely used), I just need to make sure the python module is in the working directory, enable the simulator-specific tool options to load the cocotb VPI lib and run with the MODULE env var set. The flow API will further streamline this.

So I do appreciate all the work that has been done here, but choose another path forward for cocotb integration

olofk avatar Jan 06 '23 21:01 olofk