-
Notifications
You must be signed in to change notification settings - Fork 616
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
Added migration for inferModuleReset #2571
Conversation
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.
Some tiny nits and then some food for thought
val migrateIR = new chisel3.CompileOptions { | ||
val connectFieldsMustMatch = false | ||
val declaredTypeMustBeUnbound = false | ||
val dontTryConnectionsSwapped = false | ||
val dontAssumeDirectionality = false | ||
val checkSynthesizable = false | ||
val explicitInvalidate = false | ||
val inferModuleReset = false | ||
|
||
override val migrateInferModuleReset = true | ||
} |
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.
If we're going to adopt this new technique of "migration" compile options, I think we should consider some factory methods to construct them from Strict
and NotStrict
, I'm thinking something like:
object CompileOptions {
def migrationFromStrict(migrateInferModuleReset: Boolean = false): CompileOptions = ...
}
Anyway, I don't think it should be part of this PR, but if we are to sort of standardize on this technique I think a little bit of help for users would be warranted, otherwise this is a very power user-y API.
Co-authored-by: Jack Koenig <koenig@sifive.com>
Co-authored-by: Jack Koenig <koenig@sifive.com>
Co-authored-by: Jack Koenig <koenig@sifive.com>
Co-authored-by: Jack Koenig <koenig@sifive.com>
Co-authored-by: Jack Koenig <koenig@sifive.com> (cherry picked from commit 3c6c044) # Conflicts: # core/src/main/scala/chisel3/Module.scala
Co-authored-by: Jack Koenig <koenig@sifive.com> (cherry picked from commit 3c6c044)
Contributor Checklist
docs/src
?I didn't add more comprehensive ScalaDoc. I will add that once we add migration options for all the compilation options.
Type of Improvement
API Impact
Adds another optional field to CompileOptions to migrate inferModuleReset.
Desired Merge Strategy
Squash
Release Notes
Added an optional field to CompileOptions to help users migrate the inferModuleReset compile option.
Reviewer Checklist (only modified by reviewer)
3.4.x
, [small] API extension:3.5.x
, API modification or big change:3.6.0
)?Enable auto-merge (squash)
, clean up the commit message, and label withPlease Merge
.Create a merge commit
.