Skip to content
This repository has been archived by the owner on Apr 20, 2024. It is now read-only.

Refactor barstools for new versions of things. #35

Merged
merged 11 commits into from
Feb 13, 2019
Merged

Conversation

grebe
Copy link
Contributor

@grebe grebe commented Dec 19, 2018

  • No handlebars (not being published for Scala 2.12)
  • Update for new annotations APIs
  • Do better options parsing

@grebe
Copy link
Contributor Author

grebe commented Dec 19, 2018

The BOOM test is failing because of assignment to VOID. Could someone who knows what's going on help me with that test?

Also, I've updated options parsing to simply extend the firrtl options parser. I want to do this so you can invoke barstools exactly like firrtl. If someone could provide an example fir + anno and a barstool command doing something interesting, I would like to test that my options parsing + updated annotations handling actually works.

@donggyukim
Copy link
Contributor

Is it similar to the error in #34?

@jackkoenig
Copy link
Contributor

Assignment to VOID as in it's failing in FIRRTL? Possibly a manifestation of chipsalliance/firrtl#852? (or is something just straight up not initialized?)

For options parsing, Treadle does this: https://github.com/freechipsproject/treadle/blob/b8660e87946b1d5b352462310afa5eb75908e8ca/src/main/scala/treadle/Driver.scala.

@seldridge is working on making it better, might be worth checking with him. @chick as well.

@grebe
Copy link
Contributor Author

grebe commented Dec 20, 2018

Yes, it's like #34 (number 2 with all the VOIDs). I think it's just an old version of BOOM before all the initialization checks happened.

@jwright6323
Copy link
Collaborator

@edwardcwang I think we need to settle on one format, but I don't think conf is the right one long-term, either. IMO we need a more verbose output from --repl-seq-mem (or a --repl-seq-mem-mdf e.g.)

@edwardcwang
Copy link
Member

edwardcwang commented Feb 12, 2019

For now how about we just have both options and when we figure out the "one format to rule them all", we can add an option for that? I don't think an extra option is going to hurt anybody here.

@jwright6323
Copy link
Collaborator

I am OK with that. I'll add the old option back in.

grebe and others added 6 commits February 12, 2019 15:45
- No handlebars (not being published for Scala 2.12)
- Update for new annotations APIs

Bump sbt-dependency-graph to 0.9.2 for this scala version
Also, don't rename TestHarness.
…ak as soon as any additional external/blackbox modules are added to the test harness. The test harness should detect external modules and not rename them instead of what is happening here.
Ensure backwards compatiblity by using -m for MDF input and -n for conf
input. Also fix the naming scheme for memory ports.
case ("-n" | "--macro-conf") :: value :: tail =>
parseArgs(map + (Macros -> value) + (MacrosFormat -> "conf"), costMap, forcedMemories, tail)
case ("-m" | "--macro-mdf") :: value :: tail =>
parseArgs(map + (Macros -> value) + (MacrosFormat -> "mdf"), costMap, forcedMemories, tail)
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if they specify both?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It will take the last option specified. I think this is intuitive enough behavior to leave it as is.

macros/src/main/scala/MacroCompiler.scala Outdated Show resolved Hide resolved
macros/src/main/scala/MacroCompiler.scala Show resolved Hide resolved
macros/src/main/scala/MemConf.scala Outdated Show resolved Hide resolved
macros/src/main/scala/MemConf.scala Outdated Show resolved Hide resolved
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! Thanks for the hard work

@jwright6323 jwright6323 merged commit 1f58ea1 into master Feb 13, 2019
@grebe grebe deleted the noHandlebars branch February 13, 2019 18:28
jwright6323 added a commit to jwright6323/firrtl that referenced this pull request Feb 28, 2019
This provides a data structure wrapper around the existing memory conf format
which contains both reading and writing methods, making it easier to write code
that needs to read the format.
jwright6323 added a commit to jwright6323/firrtl that referenced this pull request Feb 28, 2019
This provides a data structure wrapper around the existing memory conf format
which contains both reading and writing methods, making it easier to write code
that needs to read the format.
jwright6323 added a commit to jwright6323/firrtl that referenced this pull request Mar 5, 2019
This provides a data structure wrapper around the existing memory conf format
which contains both reading and writing methods, making it easier to write code
that needs to read the format.
mergify bot pushed a commit to chipsalliance/firrtl that referenced this pull request Mar 7, 2019
* Copy MemConf.scala from ucb-bar/barstools#35 into memlib.
This provides a data structure wrapper around the existing memory conf format
which contains both reading and writing methods, making it easier to write code
that needs to read the format.

* Add MemConf tests and use a Map[MemPort, Int] for port lists instead of a Seq[MemPort] which is a bit less fragile.
albertchen-sifive pushed a commit to albertchen-sifive/firrtl that referenced this pull request Mar 18, 2019
…ce#1041)

* Copy MemConf.scala from ucb-bar/barstools#35 into memlib.
This provides a data structure wrapper around the existing memory conf format
which contains both reading and writing methods, making it easier to write code
that needs to read the format.

* Add MemConf tests and use a Map[MemPort, Int] for port lists instead of a Seq[MemPort] which is a bit less fragile.
ucbjrl pushed a commit to chipsalliance/firrtl that referenced this pull request Mar 26, 2019
* Copy MemConf.scala from ucb-bar/barstools#35 into memlib.
This provides a data structure wrapper around the existing memory conf format
which contains both reading and writing methods, making it easier to write code
that needs to read the format.

* Add MemConf tests and use a Map[MemPort, Int] for port lists instead of a Seq[MemPort] which is a bit less fragile.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants