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

Add barstools #44

Merged
merged 2 commits into from
Feb 14, 2019
Merged

Add barstools #44

merged 2 commits into from
Feb 14, 2019

Conversation

jwright6323
Copy link
Contributor

Waiting on ucb-bar/barstools#35.

This adds barstools to project-template and changes the make targets to generate separate top and harness files, as well as generates external memories from SeqMems. By default these memories are implemented as synthesizeable flops, but this makes plugging in a VLSI flow easier.

@zhemao
Copy link
Contributor

zhemao commented Feb 13, 2019

I'm not so sure about putting tapeout-specific generation in the regular makefiles. Shouldn't these be going into tapeout-specific directories instead? So instead of modifying Makefrag and vsim/Makefile, the custom stuff should go in a tapeout/Makefile instead.

@edwardcwang
Copy link
Member

Another thought to this: since project-template is a widely used repo with many dependencies (e.g. in firesim) and barstools/Hammer is not as stable, perhaps it would make sense to use a branch of barstools once ucb-bar/barstools#35 is merged? This could enable us to be more agile on some of the passes in there.

Alternatively a hammer-passes repo may also work for faster iteration if branches on barstools is undesirable for various reasons.

If project-template was fast/easy to test, we could just PR a corresponding fix to project-template, but that's not the case since it's slow/difficult to test project-template. I think a danger in just using a revision is that it makes it hard to pull in a fix without pulling in all new changes (continuous integration).

Having said that if the interface is via CLI/JAR it may not be so bad.

@jwright6323
Copy link
Contributor Author

@zhemao The motivation of this PR is to put the bare minimum set of stuff in the Makefrag that makes everything work. "TAPEOUT" is a bit of a misnomer, that's just what the barstools project is called- all the TAPEOUT pass does is split the DUT from the test harness, which is generally useful to all types of projects. The MacroCompiler step replaces the SeqMems with external modules so that they can be replaced with SRAMs, but retains the "default" functionality of synthesizeable-register-based seq mems by generating that file as well. For VLSI flows we'd just omit that and use our own. FPGA flows could either map to FPGA resources in a similar manner or use the generated file. I suppose tapeout/Makefile would work, but it sounds like we'd be doubling the maintenance effort. We'll see what the firesim folks say; we did discuss this beforehand.

@edwardcwang What else did you want to add to barstools, specifically? I agree that we should keep the ASIC-flow stuff as separate and easy-to-iterate-on as possible, so a hammer-passes (branch of barstools) may be the right way to go there.

@edwardcwang
Copy link
Member

edwardcwang commented Feb 13, 2019

e.g. passes that extract e.g. clock inverter annotations and then generate Hammer IR. You can imagine passes like this for the hammer APIs we have. Maybe those should go in hammer-passes as you mention.

@colinschmidt
Copy link
Contributor

Given that there is no change in behavior for vsim is there still an issue @zhemao?
I think new features should always be on branches so if @edwardcwang is going to write new barstools passes they can live on a branch until they are robust enough. Although I will note that most barstools passes are not as robust as they should be since they are often nearly single-user.
I will be vary happy to have this kind of infrastructure more public and used.

Copy link
Contributor

@colinschmidt colinschmidt left a comment

Choose a reason for hiding this comment

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

LGTM. Lets see if @sagark @davidbiancolin have any thoughts

Makefrag Show resolved Hide resolved
Makefrag Outdated Show resolved Hide resolved
memories as external modules, which makes VLSI flows easier to plug in.
@jwright6323 jwright6323 changed the title [DO NOT MERGE] Add barstools Add barstools Feb 13, 2019
@jwright6323
Copy link
Contributor Author

Cool, this is ready to go now that ucb-bar/barstools#35 is done. Will wait for signoff from everyone here, though.

@zhemao
Copy link
Contributor

zhemao commented Feb 13, 2019

We don't use these makefiles for firesim, so that won't be an issue. I'm fine with merging this.

Copy link
Contributor

@zhemao zhemao left a comment

Choose a reason for hiding this comment

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

Can you fix the Makefile in verisim as well?

@edwardcwang
Copy link
Member

In general stuff living on branches tends to have continuous integration issues and discoverability issues so maybe they would go into hammer-passes just so they have a proper place to live. Maybe they can link against barstools as needed.

@colinschmidt
Copy link
Contributor

Yeah I agree with @zhemao that all makefiles should use this.

@edwardcwang I think the question is whether the passes you write should eventually live in barstools or not? If they won't ever live there then you can have your own repo and integration probably lives in your downstream *-template repo. If they eventually will be integrated into barstools its best to keep them on a branch. Its easier to have fewer repos in general.

@sagark
Copy link
Member

sagark commented Feb 13, 2019

We don't use any of this in firesim directly, so I have no problem with what's here.

bug using verilator, make the whitespace consistend in
Makefrag-verilator, explicitly name the verilog sources to match vsim,
and update verisim/Makefile to use the new source variable names
@jwright6323
Copy link
Contributor Author

@zhemao Fixed verisim/Makefile

@jwright6323 jwright6323 merged commit d97afcd into master Feb 14, 2019
@edwardcwang edwardcwang deleted the johnwright branch February 14, 2019 05:53
jerryz123 pushed a commit that referenced this pull request Apr 19, 2023
Enable overriding of source ID bits in SerialAdapter
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