search

olofk / edalize

An abstraction library for interfacing EDA tools
BSD 2-Clause "Simplified" License
636 stars 189 forks source link

Support for argtype "generic" #7

Closed GCHQDeveloper992 closed 5 years ago

GCHQDeveloper992 commented 5 years ago

I was just experimenting with FuseSoC's ability to pass arguments to the edalize backends for overriding top level parameters / generics. It appears that at present only verilog params and defines work, as none of the edalize backends include 'generic' in their argtypes list (e.g. edalize/modelsim.py line 65), or include the code that could make use of them.

As a quick test, I've written a simple VHDL counter entity whose number of bits is defined via a generic. I was hoping to be able to vary this in my modelsim simulation by using:

fusesoc sim simplecounter --bits=10

The relevant part of the associated core file:

parameters:
  bits:
    datatype : int
    description : num bits in count
    paramtype : generic

Doing this currently results in the following error:

usage: fusesoc run simplecounter_0.0.1 [-h]
fusesoc run simplecounter_0.0.1: error: unrecognised arguments: --bits=31

It looks like this happens when the parse_args() function in edalize/edatool.py is supplied with parameter values that don't meet the argtypes list for the relevant backend. I'm happy to submit an update to the modelsim backend (and possibly the Quartus one as I'm familiar with that), but I've no experience with several of the other backends which may benefit from similar code changes.

Would you like a patch that adds a limited form of 'generic' support, and possibly adds a warning in the parse_args() function when it encounters a parameter name it recognises but whose type isn't present in paramtypes for now?

benreynwar commented 5 years ago

I noticed this today as well. I'll make changes to the Quartus and Vivado and put in a pull request for that since I'll be using those backends.

I think your suggestion of adding a warning is a good one.

olofk commented 5 years ago

To be perfectly honest, I noticed none of the backends implemented generics when I did a refactoring about a year ago or so. As no one had complained, I didn't prioritize fixing this at the time. But I'm happy to see this fixed, so please bring patches

olofk commented 5 years ago

And a warning for unhandled parameter types would definitely be a good thing

olofk commented 5 years ago

Let's leave this open until at least quartus and vivado are fixed as well. There are more backends to handle, but we can open separate issues/PRs for those

olofk commented 5 years ago

Looks like we have quartus/vivado support now, so closing. Feel free to open issues or send PRs for the other backends (e.g. ghdl and ise)