Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

efinix: add synthesis options to a dictionary in the EfinityToolchain #1971

Closed

Conversation

jwise
Copy link
Contributor

@jwise jwise commented May 29, 2024

Sometimes the synthesis tool needs some help to successfully map a design, in the form of different synthesis options.

In my particular case, this didn't actually save me for my design, but it would be nice to have it available in the future if I need it again. I use this in my Platform like this:

class Platform(EfinixPlatform):
    default_clk_name   = "clk50"
    default_clk_period = 1e9/50e6

    def __init__(self, toolchain="efinity"):
        EfinixPlatform.__init__(self, "T20F256C4", _io, iobank_info=_bank_info, toolchain=toolchain)
        self.toolchain.map_opts['mode'] = 'area'

@trabucayre
Copy link
Collaborator

Hi,
Why not implementing efinity_args and efinity_argdict to allows users to specify/modify these parameters directly from the command line? See trellis toolchain and associated call from the platform?

Also one remark: using a dict and itertools allows to have a code more compact but its increase difficulties to read/update/maintain code.

@enjoy-digital
Copy link
Owner

Thanks @jwise, I agree with @trabucayre about using something closer to what is already present on other platforms (and to favor readability vs code compactness).

@trabucayre
Copy link
Collaborator

Hi @jwise.
I have written this PR, with remarks.
With this one it's possible to change synthesis parameters directly from the CLI at build time instead of having to modify the target or the platform.
It also keep the original structure, more easy to read/maintain.

Thanks

@enjoy-digital
Copy link
Owner

Thanks @jwise, @trabucayre. #2009 has been merged so we can close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants