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

Support firesim distributed elaboration #872

Closed
wants to merge 9 commits into from

Conversation

timsnyder-siv
Copy link
Contributor

Related issue: firesim/firesim#646
This is #716 re-incarnated using a local branch so that the CI will run (and rebased several months forward).

Type of change: other enhancement

Impact: rtl change & infrastructure change
Do you guys consider changes to the makefiles as 'software' change or is that target software?
RTL change removes the FireSim/firechip duplication of WithBootROM and instead

Release Notes

  • Updates sbt-assembly config so that assembly is able to merge everything into a fat jar once again.
  • Adds a message to assertion in chipyard.Generator --legacy-config parser. Error without message was confusing.
  • Squelch message about not finding fd, if it isn't found, makefiles fall back to find.
  • Adjust makefiles to support elaborating with assembly JAR and FireSim's new gen-replace-rtl-script target

@timsnyder-siv
Copy link
Contributor Author

timsnyder-siv commented May 1, 2021

I understand why the BootROM change breaks things. I'll go fix the makefiles.

Actually, it looks like the firesim changes in firesim#651 are required for the FireSim regressions that are run as part of Chipyard. For some reason, I thought that you didn't regress FireSim in Chipyard but I don't know where I got that idea.

Should I bump FireSim to point to the firesim#651 PR? If you guys use WithBootROM outside of FireSim, that may also need to be adjusted so that the bootrom file can be found in the TargetDir of the generator.

@timsnyder-siv
Copy link
Contributor Author

Ok. Now that I'm past the FireSim failures, I see the breakages I thought I might see in sims/verilator/Makefile. I'm looking for where I would need to make a change similar to https://github.com/firesim/firesim/pull/651/files#diff-51c697eb76cf079de5f54272f1dbb3f8f9a087446f3280b154b526c85a5d66a8R42-R49 in the Chipyard makefiles (copying the bootrom.img files into the target-dir for the compilation).

@abejgonzalez
Copy link
Contributor

Do you guys consider changes to the makefiles as 'software' change or is that target software?

I think this could be clearer... but I normally refer to target software.

Should I bump FireSim to point to the firesim#651 PR?

Yup!

Ok. Now that I'm past the FireSim failures, I see the breakages I thought I might see in sims/verilator/Makefile. I'm looking for where I would need to make a change similar to https://github.com/firesim/firesim/pull/651/files#diff-51c697eb76cf079de5f54272f1dbb3f8f9a087446f3280b154b526c85a5d66a8R42-R49 in the Chipyard makefiles (copying the bootrom.img files into the target-dir for the compilation).

If you want to copy the bootrom files from testchipip, you can do that either in GenerateSimFiles (in the generators/utilities area) (which is what I think you are doing right now), or you can do it in the common.mk section of the makefile (make it a pre-requisite of the main generator). I think changing GenerateSimFiles makes sense though.

common.mk Show resolved Hide resolved
@timsnyder-siv
Copy link
Contributor Author

w00t CI is passing everything except the FireSim is on master check!

Copy link
Contributor

@abejgonzalez abejgonzalez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small comments here and there. Overall looks good.

variables.mk Outdated Show resolved Hide resolved
timsnyder-siv and others added 8 commits May 5, 2021 23:56
  * only specifically identified artifacts use non-default strategies
  * fallthrough uses default strategies (that will error)
  * adds a custom strategy that prefers a non-rocketchip artifact (notRocketMergeStrategy)
     * needed for chipard/generators/utilities/src/main/resources/csrc/emulator.cc
       so that the ChipYard version is used instead of the one from RC
  * adds (useRocketMergeStrategy) that prefers the RC artifact
     * we want to use vsrc/SimDTM.v from rocketchip, not riscv-sodor

Needed for firesim/firesim#651
common.mk looks for fd but falls back to a slower option using find.
when it isn't found, don't print that it isn't found to the output because
it is okay for it not to be found.
for FireSim chipyard as top, it gets run_scala_main and run_main from chipyard's
variables.mk
It was confusing when I hit it.  A little help without having to go
sift through the scala to find the assertion goes a long way.
not needed in Chipyard, only FireSim
timsnyder-siv pushed a commit to sifive/firesim that referenced this pull request May 6, 2021
@timsnyder-siv timsnyder-siv force-pushed the support_firesim_distributed_elaboration branch from b21ae89 to f081988 Compare May 6, 2021 01:37
@joonho3020
Copy link
Contributor

Closing stale PR

@joonho3020 joonho3020 closed this Feb 4, 2024
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.

6 participants