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

[WIP] Rebar refactoring #24

Open
wants to merge 9 commits into
base: dev
Choose a base branch
from
Open

[WIP] Rebar refactoring #24

wants to merge 9 commits into from

Conversation

davidbiancolin
Copy link
Contributor

See firesim/firesim#264.

I started moving things into their proposed locations in rebar.

@alonamid
Copy link

alonamid commented Mar 26, 2019

In an ideal world, I would like everything in firechip/rebar to be simulator-agnostic. i.e. Have all the widgets and SimConfigs be in firesim, while firechip/rebar has only SoC components that are used afterwards in the VLSI flow (or are also used in verilator/vcs).
However, given that SBT is a <insert-your-favorite-profanity> build-tool that seems to not be capable of handling this type of thing, I think it is still worth having some clearer logical separation between SoC components and simulation/testHarness components. In order words, not to have them in src/main/scala/firesim, but rather have an IO or TestHarness or Endpoints or src/sim-endpoint/scala in rebar rather than put this stuff directly in src/main/scala.

Nevertheless, I still think our vision/goal should be for firechip/rebar to be simulator-agnostic.

@davidbiancolin
Copy link
Contributor Author

I'm still struggling to understand what the problem is, but it's no problem to put most of the firesim related RTL stuff in a separate package under rebar -- my preference would be src/main/scala/<package-name> as is convention as with a decent name it will be unambiguous that it's not ASIC RTL.

Perhaps a better case for this separation relates to whether or not we're going to support making firesim an optional submodule under rebar, as currently none of this will compile without it.

@alonamid
Copy link

We had the a previous discussion about making this a "monorepo" vs a "lean project template", and it seems like the majority preferred a "monorepo" where everything is integrated and not modular (I was in a minority).
My main issue here is the logical separation of components of the RTL flow, and attempting to abstract away the complexities of this ecosystem. To use terminology from an older wise grad student: "a separation of concerns" :).
I believe that not every user will want to go through the entire flow, and that in some cases users will actually have different people that are doing simulation/verification vs. vlsi stuff vs. adding a generator/accelerator component, and therefore it should be as clear as separated as possible which code segments are simulation-related, and which code segments are SoC related.
My approach to this type of separation is usually in the form of different directories, because it's the most visible. I understand the different packages are also a form of separation, but I think it's less clear than separate and distant directories because it requires understanding of the build system (which I had hoping to abstract away from users).

lazy val hardfloat = (project in rocketChipDir / "hardfloat")
.settings(
commonSettings,
crossScalaVersions := Seq("2.11.12", "2.12.4"))
Copy link

Choose a reason for hiding this comment

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

The 2.11 cross scala versions have been dropped, rocket is only 2.12

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool

build.sbt Show resolved Hide resolved
build.sbt Show resolved Hide resolved
lazy val firechip = (project in file("."))
.settings(commonSettings)
.dependsOn(boom, icenet, testchipip, sifive_blocks, midasTargetUtils, midas,
firesim % "test->test;compile->compile")
Copy link

Choose a reason for hiding this comment

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

Is the test dependency needed? I assume so, otherwise you wouldn't bother, but want to check in case this is no longer true after refactoring.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are common classes in test i extend in tests i've moved into rebar. I could make the dependency more specific.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are common test classes i extend here -- i should be able to make the dependency far more percise as it's more test:compile->test:compile right? I already forget the scope syntax.

Copy link

Choose a reason for hiding this comment

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

You could make the dep more precise but it doesn't really matter that much for test.

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.

None yet

3 participants