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

Update to Chisel 3.4.0 and FIRRTL 1.4.0 #2617

Merged
merged 1 commit into from Oct 6, 2020
Merged

Update to Chisel 3.4.0 and FIRRTL 1.4.0 #2617

merged 1 commit into from Oct 6, 2020

Conversation

jackkoenig
Copy link
Contributor

@jackkoenig jackkoenig commented Aug 22, 2020

This PR kills the FIRRTL fat jar, instead using chipsalliance/chisel#1563 to include FIRRTL as a managed dependency in SBT. It also supports toggling between source and published dependencies, if you delete .sbtopts it will use the published dependencies.

TODO

  • Add test for ability to toggle between source and published dependencies (this currently would not work because RC1 has a bug that breaks rocket-chip)
  • FIRRTL test utils

Related issue:

Type of change: other enhancement

Impact: API modification (to build API and Makefiles)

Development Phase: implementation

Release Notes

Change build flow to build FIRRTL as SBT-managed dependency and use rocket-chip fat jar in Makefiles

@jackkoenig jackkoenig added draft Used by mergify.io automated merging rules API Modification labels Aug 22, 2020
@jackkoenig
Copy link
Contributor Author

DONT MERGE because this currently points to branches of chisel3 and firrtl, will need 3.4.0-RC2 with some bug fixes and improvements before we can actually merge this.

@jackkoenig
Copy link
Contributor Author

I'm not bothering with the Wit manifest yet because this will require a bump to the api-*-sifives as well, Chisel 3.4 is incompatible with Chisel 3.3.

README.md Outdated Show resolved Hide resolved
Copy link
Member

@hcook hcook left a comment

Choose a reason for hiding this comment

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

Maybe add something to the readme about when to use/who should use ./scripts/swap-sbt-build?

@jackkoenig
Copy link
Contributor Author

Maybe add something to the readme about when to use/who should use ./scripts/swap-sbt-build?

./scripts/swap-sbt-build is only really intended for CI*, how should I clarify that?

*I guess it could be used by me or whoever when bumping Chisel and FIRRTL if it involves switching between building from source and from published artifacts but that's rare and it doesn't save much effort there.

@hcook
Copy link
Member

hcook commented Oct 1, 2020

Right, I was thinking you were using it to switch between the two. Maybe another way of framing my comment is: you are adding a discussion to the readme of how to modify .sbtopts for use with IDEs, if it exists. But does it exist by default, or not? If it does not exist by default, I think that IDE section should have further disclaimers along the lines of "most users dont have to care about the following, but...". If it does exist by default, then I think more explanation about how/why one would get rid of it to depend on the published jars is merited in some other section.

@jackkoenig
Copy link
Contributor Author

Right, it's a little subtle so I understand your confusion and I'm trying to figure out the best way to express it.

Basically, the existence of .sbtopts controls if we're building from source or published artifacts. Whether we're building from source or published artifacts is a property of a particular commit of rocket-chip, it is not something the user should be deleting or adding (that changes the versions of things being used, it's like just checking out a different commit of chisel3 today). So it may exist, or may not, in a given commit of rocket-chip. If it exists, IDE users must do something special to make the IDE work, otherwise they need not do anything except import the project.

@hcook
Copy link
Member

hcook commented Oct 2, 2020

So then do you view this more as a one way street, where after a certain point in any given branch timeline only the published artifacts are used / the file no longer exists? Or is this something that is going to get toggled back and forth on different branches depending on the workflow of the particular user (i.e. devs working on chisel and firrtl versus everyone else?)

@jackkoenig
Copy link
Contributor Author

So then do you view this more as a one way street, where after a certain point in any given branch timeline only the published artifacts are used / the file no longer exists? Or is this something that is going to get toggled back and forth on different branches depending on the workflow of the particular user

I'm honestly not sure. My hope is that it's more-or-less a one way street; master (and hopefully someday releases) always work with released versions of chisel and firrtl while weird backport or tapeout branches will likely build from source. That being said, this is the first time moving to published versions so it's hard to say how it will pan out.

@jackkoenig jackkoenig force-pushed the chisel-3-4 branch 2 times, most recently from 3a1d548 to 9431313 Compare October 5, 2020 21:05
Use sbt-sriracha for fully SBT-managed source dependencies. They are
also toggleable via JVM System Properties: sbt.sourcemode and
sbt.workspace.

Update Makefiles and build rocketchip fat jar.

Support switching between source and published chisel3 and firrtl, test
in CI. Document this flow for bumping and describe impact of this flow
on IDE use.
@jackkoenig jackkoenig changed the title Update to Chisel 3.4.0-RC3 and FIRRTL 1.4.0-RC3 Update to Chisel 3.4.0 and FIRRTL 1.4.0 Oct 5, 2020
@jackkoenig jackkoenig merged commit a7b2cd7 into master Oct 6, 2020
@jackkoenig jackkoenig deleted the chisel-3-4 branch October 6, 2020 01:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
draft Used by mergify.io automated merging rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants