-
Notifications
You must be signed in to change notification settings - Fork 653
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
Have FireSim build recipes use Chipyard configs rather than FireChip configs #695
Conversation
Docs pls |
touché. Added docs update |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't we just remove the special treatment of "WithDefaultMemModel" if we're going to add it to the config tweaks. It'd simplify a bunch of the concrete configs.
The simplest method to add this config fragments to your custom Chipyard config is through FireSim's build recipe scheme. | ||
After your FireSim environment is setup, you will define your custom build recipe in ``sims/firesim/deploy/deploy/config_build_recipes.ini``. By prepending the FireSim config fragments (separated by ``_``) to your Chipyard configuration, these config fragments will be added to your custom configuration as if they were listed in a custom Chisel config class definition. For example, if you would like to convert the Chipyard ``LargeBoomConfig`` to a FireSim simulation with a DDR3 memory model, the appropriate FireSim ``TARGET_CONFIG`` would be ``DDR3FRFCFSLLC4MB_WithDefaultFireSimBridges_WithFireSimConfigTweaks_chipyard.LargeBoomConfig``. Note that the FireSim config fragments are part of the ``firesim.firesim`` scala package, and therefore there do not need to be prefixed with the full class name, and opposed to the Chipyard config fragments which need to be prefixed with the chipyard package name. | ||
|
||
An alternative method to prepending the FireSim config fragments in the FireSim build recipe is to create a new "permanent" FireChip custom configuration, which includes the FireSim config fragments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's actually alright to put the class not in FireChip. They'd just have to change the default package to point at their own and that package would have to depend on the firechip package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in this case its better to specify the path of least resistance, which is adding to FireChip. I would try to avoid leading people down the build system dependancy chain.
We don't need most of the |
I was debating whether to remove those or not. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I'm not sure how i personally feel about removing the configs entirely. We might want to deprecate them, and keep them around for one more release cycle. In any case, that will be left to a later PR.
Related issue:
Type of change: new feature
Impact: other
Release Notes
Have FireSim build recipes use shared Chipyard configs rather than custom FireChip configs. This required updating the Chipyard phase to enable config strings to accept config fragments from several different packages.
Goes together with FireSim PR 645 firesim/firesim#645