-
Notifications
You must be signed in to change notification settings - Fork 218
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 #264
Conversation
|
||
# Include target-specific sources and input generation recipes | ||
include $(TARGET_PROJECT_MAKEFRAG) | ||
|
||
verilog: $(VERILOG) | ||
compile: $(VERILOG) | ||
|
||
# Phony targets for launching the sbt shell and running scalatests | ||
sbt: $(FIRRTL_JAR) |
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.
Do you ever mark these as phony with .PHONY
?
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.
Not this Makefile, will fix.
@@ -21,62 +21,36 @@ def isolateAllTests(tests: Seq[TestDefinition]) = tests map { test => | |||
|
|||
testGrouping in Test := isolateAllTests( (definedTests in Test).value ) | |||
|
|||
val rocketChipDir = file("target-rtl/firechip/rocket-chip") | |||
val fireChipDir = file("target-rtl/firechip") | |||
lazy val firesimAsLibrary = sys.env.get("FIRESIM_STANDALONE") == None |
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 this is fine. I've used sys.props
and -D
options when invoking sbt
b/c I thought that was a little more explicit, but environment vars are fine too.
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.
Ah, sounds good.
|
||
$(info Firesim standalone) | ||
|
||
base_dir := $(firesim_base_dir) | ||
|
||
SBT ?= sbt |
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.
Is it intentional that SBT
and related vars are not defined in the else
?
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.
Yeah, they are defined in rebar's makefrag.
Here's an initial pass at the rebar refactoring. It moves the FireSim-class targets into firechip entirely, much as it would be in a general chip project.
TODO:
You now need to
export FIRESIM_STANDALONE
to indicate firesim is being used as top. This is kinda hacky, i'm open to better ways of dealing with this, but i figure since you need to sourceme to do anything else interesting, it's not marginally more hacky.