Repackagecore rebase #1078
Merged
Repackagecore rebase #1078
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
and deal with the subsequent fallout.
Extract BaseModule, DataMirror, ExtModule, IO into their own files.
Update deprecated messages to refer to correct object
and deal with the subsequent fallout. Move Aggregate, Clock, Mem, Printf, Reg Move almost all chisel3.core definitions to chisel3 or chisel3.experimental Revive core package object to provide aliases for moved definitions. Cleanup package definitions; eliminate ambiguous implicits Move more definitions to experimental. Extract BaseModule, DataMirror, ExtModule, IO into their own files. Add EnumType definition to core package. Update deprecated messages to refer to correct object Move FixedPoint into the experimental package (but keep it in Bits.scala). Add missing implicits to core/package - compatibility Cleanup: update ScalaDoc references; remove unused imports Use common 3.2 version in deprecation warning Move Binding from core to internal.
* Turned off strong enum annotations because they weren't working with Vec indexes * Add new EnumVecAnnotation for vecs of enums and vecs of bundles with enum fields * Changed Clock's width parameter back to a fixed constant value of 1 * Fixed enum annotations for Vecs of Bundles which contain enum elements * Fixed usage of "when/otherwise" to use consistent style
Discover a working combination of aggregate usage to enable coverage of subproject testing but publish a single Jar. Use "scalastyle-test-config.xml" for scalastyle config in tests. Enable "_" in method names and accept method names ending in "_=". Re-sync scalastyle-test-config.xml with scalastyle-config.xml This should finally fix #772.
ucbjrl
force-pushed
the
repackagecore-rebase
branch
from
2312c37 to
37973e2
Compare
ucbjrl
force-pushed
the
repackagecore-rebase
branch
from
37973e2 to
8c6bde6
Compare
# Conflicts: # chiselFrontend/src/main/scala/chisel3/Aggregate.scala
Previously, including an empty aggregate in a Bundle would cause a MixedDirectionAggregateException because it has no elements and thus doesn't have a direction * Add SampleElementBinding for Vec sample elements * Add ActualDirection.Empty for bound empty aggregates
This is necessary to use ChiselEnum in aggregates where things are casted using .asTypeOf
More repackage cleanup - reduce differences with master. Cleanup chisel3 references. More chisel3 reference cleanup.
Create Chisel IR Port() in a way that Converter is happy with. Also add more extensive test suite for future-proofing. Close #1063
and deal with the subsequent fallout. Move Aggregate, Clock, Mem, Printf, Reg Move almost all chisel3.core definitions to chisel3 or chisel3.experimental Revive core package object to provide aliases for moved definitions. Cleanup package definitions; eliminate ambiguous implicits Move more definitions to experimental. Extract BaseModule, DataMirror, ExtModule, IO into their own files. Put BitPat back in chisel3.util More experimental motion - avoid multiple import definitions. Add experimental.FixedPoint alias Add EnumType definition to core package. Update deprecated messages to refer to correct object Move FixedPoint into the experimental package (but keep it in Bits.scala). Add missing implicits to core/package - compatibility Cleanup: update ScalaDoc references; remove unused imports Add Reset alias to core/package Use common 3.2 version in deprecation warning Move Binding from core to internal. Optimize imports. Repair IntelliJ's overly cleanliness. Move Bits, Data, and BitPat to chiselFrontend/src/main/scala/chisel3 and deal with the subsequent fallout. Move Aggregate, Clock, Mem, Printf, Reg Move almost all chisel3.core definitions to chisel3 or chisel3.experimental Revive core package object to provide aliases for moved definitions. Cleanup package definitions; eliminate ambiguous implicits Move more definitions to experimental. Extract BaseModule, DataMirror, ExtModule, IO into their own files. Add EnumType definition to core package. Update deprecated messages to refer to correct object Move FixedPoint into the experimental package (but keep it in Bits.scala). Add missing implicits to core/package - compatibility Cleanup: update ScalaDoc references; remove unused imports Use common 3.2 version in deprecation warning Move Binding from core to internal. Optimize imports. Merge cleanup. Remove unused import Bump core deprecation to 3.3 Move DontCare back into Data.scala inside package internal Re-indent experimental/internal package code Move code back to original files - facilitate comparison with other branches Some code motion, update imports, minimize master differences Move exceptions up to chisel3 package object - they're part of the interface. More master diff minimization. Fix minor discrepancies with repackagecore-testbed
ucbjrl
force-pushed
the
repackagecore-rebase
branch
from
8c6bde6 to
3befa30
Compare
ducky64
requested changes
Apr 25, 2019
There was a problem hiding this comment.
Some concerns looking through the changes.
One recurring question: should experimental features be put into their own file and folder (chisel3/experimental) instead of being mixed into the base (chisel3) folder? Ditto for internal; it would also make it clearer for developers what is what.
| getPortNames(name, data).toList | ||
| /** Experimental hardware construction reflection API | ||
| */ | ||
| object DataMirror { |
There was a problem hiding this comment.
Might be a good idea to move to its own file too?
| /** RHS (source) for Invalidate API. | ||
| * Causes connection logic to emit a DefInvalid when connected to an output port (or wire). | ||
| */ | ||
| private[chisel3] object InternalDontCare extends Element { |
There was a problem hiding this comment.
Can this not be internal (object aliasing is kind of what we're trying to avoid right)? Is there a reason it needs to be accessed through the package object?
There was a problem hiding this comment.
We need to ensure the object is created before it's referenced by user code.
My earlier attempt at doing so (initializeSingletons()) causes random deadlock during testing with multiple threads.
src/main/scala/chisel3/Driver.scala
Outdated
| @@ -3,18 +3,14 @@ | |||
| package chisel3 | |||
|
|
|||
| import chisel3.internal.ErrorLog | |||
| import chisel3.internal.firrtl.Converter | |||
| import chisel3.internal.firrtl.{Converter, _} | |||
There was a problem hiding this comment.
Yeah. This was an unwanted IntelliJ import rewrite that slipped through my import review. Will fix.
| @@ -77,6 +77,28 @@ object BitPat { | |||
| val len = if (x.isWidthKnown) x.getWidth else 0 | |||
| apply("b" + x.litValue.toString(2).reverse.padTo(len, "0").reverse.mkString) | |||
| } | |||
|
|
|||
| implicit class fromUIntToBitPatComparable(x: UInt) extends SourceInfoDoc { | |||
There was a problem hiding this comment.
Does this import correctly? eg, if you import chisel3._, does it automatically give you the BitPat conversions? Or maybe since this is in chisel3.util, perhaps it should work if you import chisel3.util._? But this seems to imply that one needs to import chisel3.util.BitPat._
We should also make sure we have a uniform way of defining implicit conversions, either in their object (as in this case) or in the package (in everywhere else in chisel)
There was a problem hiding this comment.
This does appear to import correctly (I copied an existing pattern), but we should discuss this at the dev meeting.
As part of its import updating process, IntelliJ converted some import statements to `import package.{object, _}`. Is this intended to show an explicit dependency on `package.object` and a further dependency on `package` implicits? Unsure. Replace these with `import package._`
# Conflicts: # src/main/scala/chisel3/package.scala # src/test/scala/chiselTests/BundleLiteralSpec.scala # src/test/scala/chiselTests/DataPrint.scala # src/test/scala/chiselTests/LiteralExtractorSpec.scala
# Conflicts: # chiselFrontend/src/main/scala/chisel3/core/Module.scala
ducky64
approved these changes
May 17, 2019
jackkoenig
pushed a commit
that referenced
this pull request
Feb 28, 2023
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Repackage core files to top-level, experimental, or internal as appropriate.
Related issue: #1006
Type of change: other enhancement
Impact: no functional change
Development Phase: implementation
Release Notes
This moves most code from the
chisel3.corepackage directly tochisel3. This requires updating most imports and consolidating references to be more consistent. It provides aliases (deprecated) for types and objects that no longer exist incore, so existing user code should work un-modified.