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

Implement FIRRTL type alias mechanism for Bundles #3445

Merged
merged 45 commits into from
Sep 8, 2023
Merged
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
175ce49
Implement type aliasing into CHIRRTL emission and simple user API
jardhu Jul 25, 2023
8fc6496
Add tests for type aliases
jardhu Jul 25, 2023
1206fe3
Fix comments in test
jardhu Jul 25, 2023
8f9b68e
Block usage of FIRRTL keywords
jardhu Jul 28, 2023
ce2acf0
Don't need to check for AliasType in aliasing algorithm anymore
jardhu Jul 28, 2023
d084eac
Use final sanitized (and disambiguated in the future) alias name inst…
jardhu Jul 28, 2023
4634da7
Move firrtl keyword place to internal package object
jardhu Jul 28, 2023
526bcd3
Merge branch 'main' into chisel-emit-type-aliases
jared-barocsi Jul 31, 2023
6682395
Fix merge error
jardhu Jul 31, 2023
d16260a
Add SourceLine locators to alias name (through BundleAlias object)
jardhu Aug 1, 2023
8f42c68
Generate modified aliases for coerced passive bundles that end up bei…
jardhu Aug 3, 2023
67069c0
Print previous declared type when aliases conflict
jardhu Aug 3, 2023
ff37c70
Save bundle value tuple instead of the info
jardhu Aug 3, 2023
cf58b6c
Prettify the previous alias's source info
jardhu Aug 7, 2023
5744a96
Don't serialize type alias source info since firtool doesn't support …
jardhu Aug 28, 2023
514df6c
Move type aliasing mechanism to HasTypeAlias trait
jardhu Aug 29, 2023
8518046
Expose stripped suffix as part of the BundleAlias object
jardhu Aug 29, 2023
add3277
Scalafmt
jardhu Aug 29, 2023
789765d
Merge branch 'main' into chisel-emit-type-aliases
jared-barocsi Aug 29, 2023
f09a0a2
Fix imports
jardhu Aug 29, 2023
28b786e
Merge branch 'chisel-emit-type-aliases' of github.com:chipsalliance/c…
jardhu Aug 29, 2023
f768286
Scalafmt
jardhu Aug 29, 2023
04d09eb
Fix remaining merge conflicts/errors
jardhu Aug 29, 2023
b325206
Fix test
jardhu Aug 29, 2023
9e39f5e
Make firrtl.Port convert function take a default empty typeAlias map …
jardhu Aug 29, 2023
c39d1f8
Revert default argument for convert and just duplicate convert for pa…
jardhu Aug 31, 2023
10d1968
Refactor aliasing code to Record.bind
jardhu Sep 5, 2023
c5519ae
Optimize aliasing: calculate flippedness through a hierarchal approach
jardhu Sep 6, 2023
8455306
Refactor aliasing code into a Builder function
jardhu Sep 6, 2023
dff42bc
Optimize aliasing: Recursively walk aliased Records only once (during…
jardhu Sep 7, 2023
549868e
Refactor things further and remove redundancies
jardhu Sep 8, 2023
f40377f
Use resolved parent direction in directionality/coercion calculation.…
jardhu Sep 8, 2023
59d8b46
Rename keyword list to illegal alias list for better clarification
jardhu Sep 8, 2023
00e7f58
Merge branch 'main' into chisel-emit-type-aliases
jared-barocsi Sep 8, 2023
012d53f
Reintroduce flipped value since it was removed by merge
jardhu Sep 8, 2023
1829b20
Add default implementation to isFlipped
jardhu Sep 8, 2023
edd6e30
Fix test expected output string
jardhu Sep 8, 2023
c496346
Rename BundleAlias class and change HasTypeAlias.aliasName type
jardhu Sep 8, 2023
4bf92c6
Add test for user-specified suffixing
jardhu Sep 8, 2023
bdfb17c
Add tests for non-Bundle Record type aliasing
jardhu Sep 8, 2023
2ec7738
Rename isFlipped to containsAFlipped
jardhu Sep 8, 2023
a1a3cd5
Fix docs, write cookbook entry
jardhu Sep 8, 2023
4f4b6d9
Scalafmt
jardhu Sep 8, 2023
d611705
Fix doc comment issue
jardhu Sep 8, 2023
33feb12
Merge branch 'main' into chisel-emit-type-aliases
jared-barocsi Sep 8, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion core/src/main/scala/chisel3/Aggregate.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,21 @@ import chisel3.experimental.dataview.{isView, reifySingleData, InvalidViewExcept
import scala.collection.immutable.{SeqMap, VectorMap}
import scala.collection.mutable.{HashSet, LinkedHashMap}
import scala.language.experimental.macros
import chisel3.experimental.{BaseModule, BundleLiteralException, OpaqueType, VecLiteralException}
import chisel3.experimental.{
BaseModule,
BundleAlias,
BundleLiteralException,
HasTypeAlias,
OpaqueType,
VecLiteralException
}
import chisel3.experimental.{SourceInfo, UnlocatableSourceInfo}
import chisel3.internal._
import chisel3.internal.Builder.pushCommand
import chisel3.internal.firrtl._
import chisel3.internal.sourceinfo.{SourceInfoTransform, VecTransform}
import chisel3.reflect.DataMirror
import _root_.firrtl.{ir => fir}

import java.lang.Math.{floor, log10, pow}
import scala.collection.mutable
Expand Down Expand Up @@ -183,6 +192,8 @@ sealed class Vec[T <: Data] private[chisel3] (gen: => T, val length: Int) extend
*/
override def typeName = s"Vec${length}_${gen.typeName}"

override def isFlipped = sample_element.isFlipped

private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection): Unit = {
this.maybeAddToParentIds(target)
binding = target
Expand Down Expand Up @@ -1018,6 +1029,12 @@ abstract class Record extends Aggregate {
}
}

/* Tracking variable for deciding Record flipped-ness. */
private[chisel3] var _isFlipped: Boolean = false

/* In the context of Records, isFlipped is assigned true if any of its children are flipped. */
override def isFlipped: Boolean = _isFlipped
jared-barocsi marked this conversation as resolved.
Show resolved Hide resolved

private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection): Unit = {
this.maybeAddToParentIds(target)
binding = target
Expand All @@ -1038,6 +1055,9 @@ abstract class Record extends Aggregate {
)
}
child.bind(ChildBinding(this), resolvedDirection)

// Update the flipped tracker based on the flipped-ness of this specific child element
_isFlipped |= child.isFlipped
}

// Check that children obey the directionality rules.
Expand All @@ -1053,6 +1073,11 @@ abstract class Record extends Aggregate {
}
}
setElementRefs()

this match {
case aliasedRecord: HasTypeAlias => Builder.setRecordAlias(aliasedRecord, resolvedDirection)
case _ =>
}
}

/** Creates a Bundle literal of this type with specified values. this must be a chisel type.
Expand Down Expand Up @@ -1463,4 +1488,5 @@ abstract class Bundle extends Record {
* the fields in the order they were defined
*/
override def toPrintable: Printable = toPrintableHelper(_elements.toList.reverse)

}
3 changes: 3 additions & 0 deletions core/src/main/scala/chisel3/Data.scala
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,9 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc {
}
}

// Whether this node has a specified flipped direction, ignoring coercion via Input/Output
jared-barocsi marked this conversation as resolved.
Show resolved Hide resolved
private[chisel3] def isFlipped: Boolean = false

// Must clone a Data if any of the following are true:
// * It has a binding
// * Its id is older than prevId (not "freshly created")
Expand Down
4 changes: 4 additions & 0 deletions core/src/main/scala/chisel3/Element.scala
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,8 @@ abstract class Element extends Data {
}
}

override def isFlipped = specifiedDirection match {
case SpecifiedDirection.Flip | SpecifiedDirection.Input => true
case _ => false
}
}
51 changes: 51 additions & 0 deletions core/src/main/scala/chisel3/experimental/HasTypeAlias.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package chisel3.experimental

import chisel3.Record

/** Wrapper object for a Record alias name. Primarily intended to provide an invocation point for source line locators, but
* also contains pertinent information to generating FIRRTL alias statements.
*
* @param id The desired name to generate an alias statement
* @param strippedSuffix In the case of forced coersion by [[Input]] or [[Output]], the string to append to the end of the
* alias name. Takes the default value of `"_stripped"`
*/
case class BundleAlias private[chisel3] (info: SourceInfo, id: String, strippedSuffix: String = "_stripped")

object BundleAlias {
def apply(id: String)(implicit info: SourceInfo): BundleAlias = BundleAlias(info, id)
def apply(id: String, strippedSuffix: String)(implicit info: SourceInfo): BundleAlias =
BundleAlias(info, id, strippedSuffix)
}

trait HasTypeAlias {
jared-barocsi marked this conversation as resolved.
Show resolved Hide resolved
this: Record =>

/** An optional FIRRTL type alias name to give to this [[Record]]. If overrided with a `Some(...)`, for instance
* `Some(BundleAlias("UserBundle"))`, this causes emission of circuit-level FIRRTL statements that declare that name for
* this bundle type:
*
* @example
* {{{
* class MyBundle extends Bundle with HasTypeAlias {
* override def aliasName = Some("UserBundle")
jared-barocsi marked this conversation as resolved.
Show resolved Hide resolved
* }
* }}}
*
* {{{
* circuit Top :
* type UserBundle = { ... }
* module Top :
* // ...
* }}}
*
* This is used as a strong hint for the generated type alias: steps like sanitization and disambiguation
* may change the resulting alias by necessity, so there is no certain guarantee that the desired name will show up in
* the generated FIRRTL.
*/
def aliasName: Option[BundleAlias] = None

// The final sanitized and disambiguated alias for this bundle, generated when aliasName is a non-empty Option.
// This is important if sanitization and disambiguation results in a changed alias,
// as the sanitized name no longer matches the user-specified alias.
private[chisel3] var finalizedAlias: Option[String] = None
}
85 changes: 82 additions & 3 deletions core/src/main/scala/chisel3/internal/Builder.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import _root_.firrtl.annotations.AnnotationUtils.validComponentName
import _root_.firrtl.AnnotationSeq
import _root_.firrtl.renamemap.MutableRenameMap
import _root_.firrtl.util.BackendCompilationUtilities._
import _root_.firrtl.{ir => fir}
import chisel3.experimental.dataview.{reify, reifySingleData}
import chisel3.internal.Builder.Prefix
import logger.LazyLogging
Expand Down Expand Up @@ -453,6 +454,13 @@ private[chisel3] class DynamicContext(
val globalNamespace = Namespace.empty
val globalIdentifierNamespace = Namespace.empty('$')

// A mapping from previously named bundles to their hashed structural/FIRRTL types, for
// disambiguation purposes when emitting type aliases
// Records are used as the key for this map to both represent their alias name and preserve
// the chisel Bundle structure when passing everything off to the Converter
private[chisel3] val aliasMap: mutable.LinkedHashMap[String, (fir.Type, SourceInfo)] =
mutable.LinkedHashMap.empty[String, (fir.Type, SourceInfo)]

// Ensure imported Definitions emit as ExtModules with the correct name so
// that instantiations will also use the correct name and prevent any name
// conflicts with Modules/Definitions in this elaboration
Expand Down Expand Up @@ -536,8 +544,12 @@ private[chisel3] object Builder extends LazyLogging {

def globalNamespace: Namespace = dynamicContext.globalNamespace
def globalIdentifierNamespace: Namespace = dynamicContext.globalIdentifierNamespace
def components: ArrayBuffer[Component] = dynamicContext.components
def annotations: ArrayBuffer[ChiselAnnotation] = dynamicContext.annotations

def aliasMap: mutable.LinkedHashMap[String, (fir.Type, SourceInfo)] =
dynamicContext.aliasMap

def components: ArrayBuffer[Component] = dynamicContext.components
def annotations: ArrayBuffer[ChiselAnnotation] = dynamicContext.annotations

def contextCache: BuilderContextCache = dynamicContext.contextCache

Expand Down Expand Up @@ -857,6 +869,57 @@ private[chisel3] object Builder extends LazyLogging {
renames
}

def setRecordAlias(record: Record with HasTypeAlias, parentDirection: SpecifiedDirection): Unit = {
val alias = record.aliasName.flatMap { candidateAlias =>
{
val sourceInfo = candidateAlias.info

// If the aliased bundle is coerced and it has flipped signals, then they must be stripped
val isCoerced = parentDirection match {
case SpecifiedDirection.Input | SpecifiedDirection.Output => true
case other => false
}
val isStripped = isCoerced && record.isFlipped

// The true alias, after sanitization and (TODO) disambiguation
val alias = sanitize(s"${candidateAlias.id}${if (isStripped) candidateAlias.strippedSuffix else ""}")
// Filter out (TODO: disambiguate) FIRRTL keywords that cause parser errors if used
if (illegalTypeAliases.contains(alias)) {
Builder.error(
s"Attempted to use an illegal word '$alias' for a type alias. Chisel does not automatically disambiguate these aliases at this time."
)(sourceInfo)

None
} else {
val tpe = Converter.extractType(record, isCoerced, sourceInfo, true, true, aliasMap.keys.toSeq)
// If the name is already taken, check if there exists a *structurally equivalent* bundle with the same name, and
// simply error (TODO: disambiguate that name)
if (
Builder.aliasMap.contains(alias) &&
Builder.aliasMap.get(alias).exists(_._1 != tpe)
) {
// Get full structural map value
val recordValue = Builder.aliasMap.get(alias).get
// Conflict found:
error(
s"Attempted to redeclare an existing type alias '$alias' with a new Record structure:\n'$tpe'.\n\nThe alias was previously defined as:\n'${recordValue._1}${recordValue._2
.makeMessage(" " + _)}"
)(sourceInfo)

None
} else {
if (!Builder.aliasMap.contains(alias)) {
Builder.aliasMap.put(alias, (tpe, sourceInfo))
}

Some(alias)
}
}
}
}
record.finalizedAlias = alias
}

private[chisel3] def build[T <: BaseModule](
f: => T,
dynamicContext: DynamicContext,
Expand All @@ -883,7 +946,23 @@ private[chisel3] object Builder extends LazyLogging {
errors.checkpoint(logger)
logger.info("Done elaborating.")

(Circuit(components.last.name, components.toSeq, annotations.toSeq, makeViewRenameMap, newAnnotations.toSeq), mod)
val typeAliases = aliasMap.flatMap {
// Discard the previously-computed FIRRTL type as the converter will now have alias information
case (name, (underlying: fir.Type, info: SourceInfo)) => Some(DefTypeAlias(info, underlying, name))
case _ => None
}.toSeq

(
Circuit(
components.last.name,
components.toSeq,
annotations.toSeq,
makeViewRenameMap,
newAnnotations.toSeq,
typeAliases
),
mod
)
}
}
initializeSingletons()
Expand Down
Loading