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.

Sign up for GitHub

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

Jump to bottom

Module Hierarchy Refactor #469

Merged
merged 6 commits into from
Apr 14, 2017
Merged

Module Hierarchy Refactor #469

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b1a1b25
Module hierarchy refactor
ducky64 Jan 27, 2017
62bd0e6
Address review comments
ducky64 Mar 21, 2017
398e252
RawModule tests, fixes to allow RawModule to instantiate
ducky64 Mar 21, 2017
3c31088
Add MultiIO inheritance-composition test / example
ducky64 Mar 22, 2017
51cdfbe
Remove comment
ducky64 Mar 22, 2017
e6f405e
Allow top-level RawModule
ducky64 Mar 30, 2017
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
4 changes: 2 additions & 2 deletions chiselFrontend/src/main/scala/chisel3/core/Attach.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ object attach { // scalastyle:ignore object.name
AttachException(": Conditional attach is not allowed!")

// Actual implementation
private[core] def impl(elts: Seq[Analog], contextModule: Module)(implicit sourceInfo: SourceInfo): Unit = {
private[core] def impl(elts: Seq[Analog], contextModule: UserModule)(implicit sourceInfo: SourceInfo): Unit = {
if (Builder.whenDepth != 0) throw ConditionalAttachException

// TODO Check that references are valid and can be attached
Expand All @@ -35,7 +35,7 @@ object attach { // scalastyle:ignore object.name
*/
def apply(elts: Analog*)(implicit sourceInfo: SourceInfo): Unit = {
try {
impl(elts, Builder.forcedModule)
impl(elts, Builder.forcedUserModule)
} catch {
case AttachException(message) =>
throwException(elts.mkString("Attaching (", ", ", s") failed @$message"))
Expand Down
10 changes: 5 additions & 5 deletions chiselFrontend/src/main/scala/chisel3/core/BiConnect.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ object BiConnect {
* during the recursive decent and then rethrow them with extra information added.
* This gives the user a 'path' to where in the connections things went wrong.
*/
def connect(sourceInfo: SourceInfo, connectCompileOptions: CompileOptions, left: Data, right: Data, context_mod: Module): Unit =
def connect(sourceInfo: SourceInfo, connectCompileOptions: CompileOptions, left: Data, right: Data, context_mod: UserModule): Unit =
(left, right) match {
// Handle element case (root case)
case (left_a: Analog, right_a: Analog) =>
Expand Down Expand Up @@ -121,12 +121,12 @@ object BiConnect {

// This function checks if element-level connection operation allowed.
// Then it either issues it or throws the appropriate exception.
def elemConnect(implicit sourceInfo: SourceInfo, connectCompileOptions: CompileOptions, left: Element, right: Element, context_mod: Module): Unit = {
def elemConnect(implicit sourceInfo: SourceInfo, connectCompileOptions: CompileOptions, left: Element, right: Element, context_mod: UserModule): Unit = {
Copy link
Contributor

Choose a reason for hiding this comment

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

You can drastically simplify this function (at least for the sink := source behavior variant) by just asserting that the sink is writeable and the source is readable then issuing. Then, you don't need to match on all these cases and subcases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this code improvement is orthogonal to the module refactor, this can be done as a follow-on PR.

import Direction.{Input, Output} // Using extensively so import these
// If left or right have no location, assume in context module
// This can occur if one of them is a literal, unbound will error previously
val left_mod: Module = left.binding.location.getOrElse(context_mod)
val right_mod: Module = right.binding.location.getOrElse(context_mod)
val left_mod: BaseModule = left.binding.location.getOrElse(context_mod)
val right_mod: BaseModule = right.binding.location.getOrElse(context_mod)

val left_direction: Option[Direction] = left.binding.direction
val right_direction: Option[Direction] = right.binding.direction
Expand Down Expand Up @@ -250,7 +250,7 @@ object BiConnect {

// This function checks if analog element-level attaching is allowed
// Then it either issues it or throws the appropriate exception.
def analogAttach(implicit sourceInfo: SourceInfo, left: Analog, right: Analog, contextModule: Module): Unit = {
def analogAttach(implicit sourceInfo: SourceInfo, left: Analog, right: Analog, contextModule: UserModule): Unit = {
// Error if left or right is BICONNECTED in the current module already
for (elt <- left :: right :: Nil) {
elt.biConnectLocs.get(contextModule) match {
Expand Down
10 changes: 5 additions & 5 deletions chiselFrontend/src/main/scala/chisel3/core/Binder.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,24 +41,24 @@ case object LitBinder extends Binder[LitBinding] {
def apply(in: UnboundBinding) = LitBinding()
}

case class MemoryPortBinder(enclosure: Module) extends Binder[MemoryPortBinding] {
case class MemoryPortBinder(enclosure: UserModule) extends Binder[MemoryPortBinding] {
def apply(in: UnboundBinding) = MemoryPortBinding(enclosure)
}

case class OpBinder(enclosure: Module) extends Binder[OpBinding] {
case class OpBinder(enclosure: UserModule) extends Binder[OpBinding] {
def apply(in: UnboundBinding) = OpBinding(enclosure)
}

// Notice how PortBinder uses the direction of the UnboundNode
case class PortBinder(enclosure: Module) extends Binder[PortBinding] {
case class PortBinder(enclosure: BaseModule) extends Binder[PortBinding] {
def apply(in: UnboundBinding) = PortBinding(enclosure, in.direction)
}

case class RegBinder(enclosure: Module) extends Binder[RegBinding] {
case class RegBinder(enclosure: UserModule) extends Binder[RegBinding] {
def apply(in: UnboundBinding) = RegBinding(enclosure)
}

case class WireBinder(enclosure: Module) extends Binder[WireBinding] {
case class WireBinder(enclosure: UserModule) extends Binder[WireBinding] {
def apply(in: UnboundBinding) = WireBinding(enclosure)
}

96 changes: 29 additions & 67 deletions chiselFrontend/src/main/scala/chisel3/core/Binding.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,6 @@ object Binding {
case unbound @ UnboundBinding(_) => {
element.binding = binder(unbound)
}
// If autoIOWrap is enabled and we're rebinding a PortBinding, just ignore the rebinding.
case portBound @ PortBinding(_, _) if (!(forcedModule.compileOptions.requireIOWrap) && binder.isInstanceOf[PortBinder]) =>
case binding => throw AlreadyBoundException(binding.toString)
}
)
Expand All @@ -118,68 +116,32 @@ object Binding {

// Excepts if any root element is unbound and thus not on the hardware graph
def checkSynthesizable(target: Data, error_prelude: String): Unit = {
// This is called if we support autoIOWrap
def elementOfIO(element: Element): Boolean = {
element._parent match {
case None => false
case Some(x: Module) => {
// Have we defined the IO ports for this module? If not, do so now.
if (!x.ioDefined) {
x.computePorts
element.binding match {
case SynthesizableBinding() => true
case _ => false
}
} else {
// io.flatten eliminates Clock elements, so we need to use io.allElements
val ports = x.io.allElements
val isIOElement = ports.contains(element) || element == x.clock || element == x.reset
isIOElement
}
}
}
}

// Diagnose a binding error caused by a missing IO() wrapper.
// element is the element triggering the binding error.
// Returns true if the element is a member of the module's io but ioDefined is false.
def isMissingIOWrapper(element: Element): Boolean = {
element._parent match {
case None => false
case Some(x: Module) => {
// If the IO() wrapper has been executed, it isn't missing.
if (x.ioDefined) {
false
} else {
// TODO: We should issue the message only once, and if we get here,
// we know the wrapper is missing, whether or not the element is a member of io.
// But if it's not an io element, we want to issue the complementary "unbound" error.
// Revisit this when we collect error messages instead of throwing exceptions.
// The null test below is due to the fact that we may be evaluating the arguments
// of the IO() wrapper itself.
(x.io != null) && x.io.flatten.contains(element)
}
}
}
}

try walkToBinding(
target,
element => element.binding match {
case SynthesizableBinding() => {} // OK
case binding =>
// The following kludge is an attempt to provide backward compatibility
// It should be done at at higher level.
if ((forcedModule.compileOptions.requireIOWrap || !elementOfIO(element))) {
// Generate a better error message if this is a result of a missing IO() wrapper.
if (isMissingIOWrapper(element)) {
throw MissingIOWrapperException
} else {
throw NotSynthesizableException
element => {
// Compatibility mode to automatically wrap ports in IO
// TODO: remove me, perhaps by removing Bindings checks from compatibility mode
element._parent match {
case Some(x: BaseModule) => x._autoWrapPorts
case _ =>
}
// Actual binding check
element.binding match {
case SynthesizableBinding() => // OK
case binding => {
// Attempt to diagnose common bindings issues, like forgot to wrap IO(...)
element._parent match {
case Some(x: LegacyModule) =>
// null check in case we try to access io before it is defined
if ((x.io != null) && (x.io.flatten contains element)) {
throw MissingIOWrapperException
}
case _ =>
}
} else {
Binding.bind(element, PortBinder(element._parent.get), "Error: IO")
// Fallback generic exception
throw NotSynthesizableException
}
}
}
)
catch {
Expand All @@ -190,7 +152,7 @@ object Binding {

// Location refers to 'where' in the Module hierarchy this lives
sealed trait Binding {
def location: Option[Module]
def location: Option[BaseModule]
def direction: Option[Direction]
}

Expand All @@ -202,7 +164,7 @@ sealed trait UnconstrainedBinding extends Binding {
// A constrained binding can only be read/written by specific modules
// Location will track where this Module is
sealed trait ConstrainedBinding extends Binding {
def enclosure: Module
def enclosure: BaseModule
def location = Some(enclosure)
}

Expand All @@ -224,19 +186,19 @@ sealed trait SynthesizableBinding extends Binding
case class LitBinding() // will eventually have literal info
extends SynthesizableBinding with UnconstrainedBinding with UndirectionedBinding

case class MemoryPortBinding(enclosure: Module)
case class MemoryPortBinding(enclosure: UserModule)
extends SynthesizableBinding with ConstrainedBinding with UndirectionedBinding

// TODO(twigg): Ops between unenclosed nodes can also be unenclosed
// However, Chisel currently binds all op results to a module
case class OpBinding(enclosure: Module)
case class OpBinding(enclosure: UserModule)
extends SynthesizableBinding with ConstrainedBinding with UndirectionedBinding

case class PortBinding(enclosure: Module, direction: Option[Direction])
case class PortBinding(enclosure: BaseModule, direction: Option[Direction])
extends SynthesizableBinding with ConstrainedBinding

case class RegBinding(enclosure: Module)
case class RegBinding(enclosure: UserModule)
extends SynthesizableBinding with ConstrainedBinding with UndirectionedBinding

case class WireBinding(enclosure: Module)
case class WireBinding(enclosure: UserModule)
extends SynthesizableBinding with ConstrainedBinding with UndirectionedBinding
2 changes: 1 addition & 1 deletion chiselFrontend/src/main/scala/chisel3/core/Bits.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1047,7 +1047,7 @@ final class Analog private (width: Width) extends Element(width) {
// Used to enforce single bulk connect of Analog types, multi-attach is still okay
// Note that this really means 1 bulk connect per Module because a port can
// be connected in the parent module as well
private[core] val biConnectLocs = mutable.Map.empty[Module, SourceInfo]
private[core] val biConnectLocs = mutable.Map.empty[UserModule, SourceInfo]

// Define setter/getter pairing
// Analog can only be bound to Ports and Wires (and Unbound)
Expand Down
Loading