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

Chisel 3.4, FIRRTL 1.4 and Rocket Chip November Bump #719

Merged
merged 49 commits into from
Dec 13, 2020
Merged

Conversation

abejgonzalez
Copy link
Contributor

@abejgonzalez abejgonzalez commented Nov 16, 2020

Related PR: #717 , ucb-bar/barstools#87, firesim/firesim#658 ucb-bar/cva6-wrapper#10 ucb-bar/gemmini#37 ucb-bar/hwacha#23 ucb-bar/riscv-sodor#59 ucb-bar/sha3#27 ucb-bar/dsptools#217 ucb-bar/testchipip#107 ucb-bar/barstools#87

Type of change: other enhancement

Impact: all RTL generation

Release Notes
Thanks @timsnyder-siv for helping out with this and the FireSim side of this.

This is a local version of #717 that bumps multiple repos to use Chisel 3.4 / FIRRTL 1.4 / RC Nov.

Changes

  • Switch JAVA_ARGS to JAVA_OPTS (done so that people could use the sbt bash script instead of just the launcher and it still works with Chipyard)
  • Split SBT_OPTS from JAVA_OPTS (clearer distinction between the two and also supports the sbt bash script)
  • Modifications to CI for *_OPTS env. vars
  • Bump virtually every sub-module
  • Deprecate bloop support
  • Support SBT thin client with export ENABLE_SBT_THIN_CLIENT=1
  • build.sbt refactoring including RC changes
  • Helper make targets to launch console (sbt) and shutdown/start thin server (<start/shutdown>-sbt-server)
  • Bump to SBT 1.4.4 and bump plugins
  • Use ; X; Y; Z; syntax to run multiple SBT commands

timsnyder-siv and others added 4 commits November 11, 2020 18:57
Note: firesim and barstools point to commits in the sifive forks of those repos
I didn't update the URL in .gitmodules because I'm not sure how that works in a PR
(because you wouldn't really want to merge sync'ing to the sifive repo).

Requires: ucb-bar/barstools#92 and firesim/firesim#658

The version of rocket-chip, chisel3 and firrtl is chosen here because it is
the latest known to pass my tests.  You will likely want to bump further.
@abejgonzalez
Copy link
Contributor Author

abejgonzalez commented Nov 16, 2020

I'm not exactly sure how FIRRTL is built *yet since the JAR isn't built 1st... However, these changes seem to make it decently far into the build until chisel-testers fails.

@timsnyder-siv
Copy link
Contributor

I'm not exactly sure how FIRRTL is built *yet since the JAR isn't built 1st... However, these changes seem to make it decently far into the build until chisel-testers fails.

Looks like you've started figuring this out because I see sbt-sriracha added to the project/plugins.sbt. However, I don't see you using the .sourceDependency() method that sbt-sriracha adds to sbtProject. See https://github.com/sbt/sbt-sriracha#usage

Let me know if you'd like some help getting the sbt-sriracha stuff ironed out. I'd be happy to submit another PR to merge into your local-chisel34 branch. I talked to @davidbiancolin about this yesterday afternoon. Wasn't sure if you guys would want to switch over to using it and didn't include it in what I had in #717

@abejgonzalez
Copy link
Contributor Author

I'm not exactly sure how FIRRTL is built *yet since the JAR isn't built 1st... However, these changes seem to make it decently far into the build until chisel-testers fails.

Looks like you've started figuring this out because I see sbt-sriracha added to the project/plugins.sbt. However, I don't see you using the .sourceDependency() method that sbt-sriracha adds to sbtProject. See https://github.com/sbt/sbt-sriracha#usage

Let me know if you'd like some help getting the sbt-sriracha stuff ironed out. I'd be happy to submit another PR to merge into your local-chisel34 branch. I talked to @davidbiancolin about this yesterday afternoon. Wasn't sure if you guys would want to switch over to using it and didn't include it in what I had in #717

Thanks for the look. I was just quickly trying to hack stuff up so this may not be the cleanest bump at the moment.

Funny enough, I wasn't able to duplicate the sbt-sriracha stuff that Rocket does in CY. I would get errors stating that chisel couldn't be found in the main CY directory. This was after pointing the -Dsbt.workspace to $PWD/tools/. If you revert to 9d9813f you can replicate the issue.

In any case, even though CY is not using sbt-sriracha, it needs to be added in CY's plugins since it is used in FIRRTL/Chisel/RC (I believe that SBT doesn't inherit sub-project plugins... though my SBT knowledge is lacking).

For now this most recent PR seems to build Chisel/FIRRTL properly from source so maybe the issue is using (project in vs ProjectRef? I'm definitely happy to take an external PR if you have a nicer solution!

@jackkoenig
Copy link
Contributor

Funny enough, I wasn't able to duplicate the sbt-sriracha stuff that Rocket does in CY. I would get errors stating that chisel couldn't be found in the main CY directory. This was after pointing the -Dsbt.workspace to $PWD/tools/.

How do you invoke SBT? SBT only picks up .sbtopts if you invoke it via the sbt shell script. Rocket Chip has logic to handle this in the Makefiles since it invokes SBT via java -jar sbt-launch.jar: https://github.com/chipsalliance/rocket-chip/blob/b5d797a435e04400281a886c72ee1cfb09e90d88/Makefrag#L28

In any case, even though CY is not using sbt-sriracha, it needs to be added in CY's plugins since it is used in FIRRTL/Chisel/RC (I believe that SBT doesn't inherit sub-project plugins... though my SBT knowledge is lacking).

This is correct

For now this most recent PR seems to build Chisel/FIRRTL properly from source so maybe the issue is using (project in vs ProjectRef?

Make sure to check your classpaths to see if you're getting the Chisel you think you're getting. eg.

$ sbt
> projects
# Will list all projects
> show <project> / Compile / dependencyClasspath

@abejgonzalez
Copy link
Contributor Author

abejgonzalez commented Nov 17, 2020

How do you invoke SBT? SBT only picks up .sbtopts if you invoke it via the sbt shell script. Rocket Chip has logic to handle this in the Makefiles since it invokes SBT via java -jar sbt-launch.jar: https://github.com/chipsalliance/rocket-chip/blob/b5d797a435e04400281a886c72ee1cfb09e90d88/Makefrag#L28

You can see the SBT invocation in the variables.mk file. I follow what RC does for now by checking on if .sbtopts exists and adding it to the cmdline invocation. Eventually, I need to check that bloop still works, but Ill get there eventually.

Make sure to check your classpaths to see if you're getting the Chisel you think you're getting. eg.

$ sbt
> projects
# Will list all projects
> show <project> / Compile / dependencyClasspath

I will look into this to double-check... but I'm pretty sure it's building the right FIRRTL/Chisel since it is giving the compile warnings for the tools/{chisel3,firrtl} repositories. Here is a quick screenshot:

image

.gitmodules Outdated
@@ -76,7 +76,7 @@
url = https://github.com/ucb-bar/dsptools.git
[submodule "tools/chisel-testers"]
path = tools/chisel-testers
url = https://github.com/freechipsproject/chisel-testers.git
url = https://github.com/abejgonzalez/chisel-testers.git
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switched to my own fork until chisel-testers is updated.

@abejgonzalez
Copy link
Contributor Author

This is able to at least elaborate a the default RocketConfig. However, this errors during the FIRRTL compilation with:

image

Ill continue trying to look into why things fail when I get the chance.

@abejgonzalez abejgonzalez changed the title Bump Chisel 3.4 Chisel 3.4, FIRRTL 1.4 and Rocket Chip November Bump Nov 20, 2020
@timsnyder-siv
Copy link
Contributor

@abejgonzalez can you share the commands you're using in Chipyard so that I can reproduce the error you're running into? Are you using 'make' commands or just doing things at the SBT prompt right now?

@abejgonzalez
Copy link
Contributor Author

abejgonzalez commented Nov 20, 2020

@abejgonzalez can you share the commands you're using in Chipyard so that I can reproduce the error you're running into? Are you using 'make' commands or just doing things at the SBT prompt right now?

Things are a bit in flux right now. Looks like some things were getting cached so building from a fresh Chipyard repo would get different errors. Im trying to track down some things as we speak. However, in the meantime, the basic steps are:

  1. Clone Chipyard
  2. Checkout this branch
  3. ./script/init-submodules-no-riscv-tools.sh
  4. Add RISCV toolchain to PATH/LD_LIBRARY_PATH/RISCV
  5. make -C sims/vcs (or sims/verilator ...)

build.sbt Outdated Show resolved Hide resolved
Co-authored-by: Abraham Gonzalez <abe.j.gonza@gmail.com>
Copy link
Contributor

@davidbiancolin davidbiancolin left a comment

Choose a reason for hiding this comment

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

I don't really understand the rationale for some of the SBT changes -- some comments should be added to explain intent, particularly regarding the ANTLR change, and the key-lookup for library dependencies.

@davidbiancolin
Copy link
Contributor

Other than that, this LGTM.

@abejgonzalez
Copy link
Contributor Author

I don't really understand the rationale for some of the SBT changes -- some comments should be added to explain intent, particularly regarding the ANTLR change, and the key-lookup for library dependencies.

Brief Explanations / Reasoning on build.sbt changes:

  • Removing commonSettings library dependencies: Most of these libraries are included in the sub-projects now so this is now unneeded. Additionally, some of these libdeps were needed since RC/FIRRTL/Chisel was dropped using the allDependencies in the past so their libdeps would not be inherited.
  • Change to allDependencies: This is just a more selective version of the prior allDependencies so that only the major projects are dropped instead of every project (since all Berkeley projects have a edu.berkeley.cs).
  • Removing conditionalDependsOn: Replaced in favor of sbt-sriracha which is what RC uses. Instead of -DROCKET_USE_MAVEN you now use the -Dsbt.sourcemode= flag.
  • Chisel/FIRRTL projects: This uses sbt-sriracha to choose between the ProjectRef version of the project or the libdep from Maven. Right now Chipyard uses the ProjectRef. Additionally, both projects store their libdeps so that they can be added to downstream projects (important because the allDependencies task drops these projects from the dependencies and thus their libdeps are also dropped). FIRRTL's libdeps are a bit weird, since it uses the ANTLR plugin, if the antlr4 libdep is included without setting up the plugin then it will break.
  • FIRRTLInterpreter/Treadle/ChiselTesters/RC: They use the Chisel/FIRRTL libdeps and also store their own libdeps.
  • Tapeout: The handlebars libdep isn't used.
  • Rest: Hopefully straightforward. Uses sourceDependency for sbt-sriracha, adds libdeps for major repos that are dropped, if was a freshProject then move the libdeps from the subproject into this file.

Copy link
Contributor

@davidbiancolin davidbiancolin left a comment

Choose a reason for hiding this comment

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

LGTM let's ship this

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.

5 participants