-
Notifications
You must be signed in to change notification settings - Fork 602
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
Chisel stage #1004
Chisel stage #1004
Conversation
a3d2f19
to
83e17a0
Compare
6888507
to
0cfb04d
Compare
5d169ae
to
2d6df13
Compare
ce0b2da
to
9aa76dd
Compare
Note: this is causing testers to fail on:
|
I am out today but will try and dig into this tonight |
This looks like a test set-up error. It is failing because VCS is not present. I'll ask @ucbjrl about it this morning |
Looking at this. There's an annoyance in that the old |
c769e1b
to
6a1c44c
Compare
Following up on the previous comment: the specific issue stems from Driver.scala#L236. Inside This is fixed by adding a couple of additional phases. In the new Corollary: mutable state is the root of all evil. Addendum: if a |
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 looks very good. Tests seem clean
Lot's of nitty gritty details
Some errors and warnings in scaladoc
a few other scala style
src/main/scala/chisel3/stage/phases/AddImplicitOutputAnnotationFile.scala
Outdated
Show resolved
Hide resolved
src/main/scala/chisel3/stage/phases/AddImplicitOutputFile.scala
Outdated
Show resolved
Hide resolved
7c890d9
to
4540500
Compare
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.
Looks good to me. All my nits are cleaned up.
@jackkoenig @ducky64 either of you want to weigh in here too?
|
||
def toAnnotations: AnnotationSeq = | ||
(if (!runFirrtlCompiler) { Seq(NoRunFirrtlCompilerAnnotation) } else { Seq() }) ++ | ||
(if (printFullStackTrace) { Some(PrintFullStackTraceAnnotation) } else { 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 didn't realize you can ++
Option
but it makes sense, cool!
This adds the following FIRRTL Annotations to Chisel: - NoRunFirrtlCompilerAnnotation - PrintFullStackTraceAnnotation - ChiselGeneratorAnnotation - ChiselCircuitAnnotation - ChiselOutputFileAnnotation This includes tests for ChiselGeneratorAnnotation as this Annotation is able to be constructed from a String and to elaborate itself. Co-Authored-By: Schuyler Eldridge <schuyler.eldridge@ibm.com> Co-Authored-By: chick <chick@qrhino.com> Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Co-Authored-By: Schuyler Eldridge <schuyler.eldridge@ibm.com> Co-Authored-By: chick <chick@qrhino.com> Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Co-Authored-By: Schuyler Eldridge <schuyler.eldridge@ibm.com> Co-Authored-By: chick <chick@qrhino.com> Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
This adds an Elaborate Phase that expands ChiselGeneratorAnnotations into ChiselCircuitAnnotations and deletes the original. Co-Authored-By: Schuyler Eldridge <schuyler.eldridge@ibm.com> Co-Authored-By: chick <chick@qrhino.com> Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
This coalesces three distinct operations into one Convert Phase: 1. Chisel Circuit to FIRRTL Circuit (CHIRRTL) conversion 2. Conversion of Chisel Annotations to FIRRTL Annotations 3. Generation of RunFirrtlTransformAnnotations Co-Authored-By: Schuyler Eldridge <schuyler.eldridge@ibm.com> Co-Authored-By: chick <chick@qrhino.com> Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
This adds an Emitter Phase that writes a ChiselCircuitAnnotation to a file if a ChiselOutputFileAnnotation is present. Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Co-Authored-By: Schuyler Eldridge <schuyler.eldridge@ibm.com> Co-Authored-By: chick <chick@qrhino.com> Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@gmail.com>
This adds ChiselStage, a reimplementation of chisel3.Driver as a firrtl.options.Stage. This is simplistically described as a pipeline of Phases. Co-Authored-By: Schuyler Eldridge <schuyler.eldridge@ibm.com> Co-Authored-By: chick <chick@qrhino.com> Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
This includes phases necessary to provide backwards compatibility with the old Chisel3 Driver. These are placed in a DriverCompatibility object inside chisel3.stage.phases. The following four phases are included: - AddImplicitOutputFile (from a TopNameAnnotation) - AddImplicitOutputAnnotationFile phase - DisableFirrtlStage (to disable ChiselStage running FirrtlStage) - MutateOptionsManager (to update options after ChiselStage) - ReEnableFirrtlStage (to renable FirrtlStage if needed) Additionally, this adds a view of a ChiselExecutionResult for providing the legacy return type of the Chisel Driver. Co-Authored-By: Schuyler Eldridge <schuyler.eldridge@ibm.com> Co-Authored-By: chick <chick@qrhino.com> Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Adds a method to enable conversion from ChiselExecutionOptions back to an AnnotationSeq. Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
This converts the original chisel3.Driver to use chisel3.stage.ChiselStage. This is implemented in the following way: 1. ExecutionOptions are converted to an AnnotationSeq 2. The AnnotationSeq is preprocessed using phases contained in the Chisel DriverCompatibility objects. One of these *disables* the execution of FirrtlStage by ChiselStage. 3. ChiselStage runs on the preprocessed AnnotationSeq 4. The input ExecutionOptionsManager is mutated based on the output of ChiselStage. 5. The FIRRTL stage is re-enabled if it's supposed to run and selected FIRRTL DriverCompatibility phases run. 6. FirrtlStage runs 7. The output AnnotationSeq is "viewed" as a ChiselExecutionResult This modifies the original DriverSpec to make it more verbose with the addition of info statements. The functionality of the DriverSpec is unmodified. Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Add "mverilog" Compiler Option, MinimumVerilogEmitter
Related issue:
Type of change: other enhancement
Impact: API addition (no impact on existing code)
Development Phase: implementation
This PR provides a reimplementation of the Chisel Driver as a
Stage
. This involves three major additions/modifications:chisel3.stage
package that implementsChiselStage
chisel3.stage
chisel3.Driver
to useChiselStage
with a compatibility layerFor a more exhaustive introduction to Stage/Phase, see chipsalliance/firrtl#1005.
Todo
ChiselExecutionResultView
implicit object
and can't be deprecated. Instead it's nowprivate [chisel3]
Structure
This PR
xOptions
class (which is not a case class)xOptions
ExecutionOptions
to reconstruct anAnnotationSeq
chisel3.stage.phases.Checks
chisel3.stage.phases.Elaborate
chisel3.stage.phases.Convert
chisel3.stage.phases.Emitter
chisel3.stage.phases.AddImplicitOutputFile
chisel3.stage.phases.AddImplicitOutputAnnotationFile
ChiselStage
Stage
Anecdotally, this process is non-linear. I did this by incrementally porting the old Driver in pieces and moving whatever functionality I could into it when I had it. Git's
--squash
and--fixup
help a lot for keeping the history sane when you do rebase. Additionally, writing tests as you go is tedious, but likely necessary.Most of the porting difficulty is (1) deciding, without constraints, what you want the new stage to do and (2) writing the compatibility layer that interfaces this with the old Driver.
There are several temptations here that I think should be avoided:
chisel3.Driver
callfirrtl.Driver
). This causes the old Driver and the new Stage to use different code paths. While not insurmountable, this would seem to introduce the potential for subtle bugs between the two versions.Most of difficulty in not yielding to these temptations is dealing with situations where the old Driver does not do things in a linear, pipe-like order. E.g., the old Chisel Driver would do elaboration and then continually refer back to the elaborated Chisel circuit. The stage doesn't need to do this as it's moving directly towards a CHIRRTL circuit or to run the FIRRTL compiler. However, the compatibility wrapper has to reconstruct what happened. This PR gets around this, kludgily, by viewing deleted annotations. This isn't really advised, but for a compatibility layer it's passable.
Depends on:
Includes:
Release Notes