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

Added disallowIOCreation as a public API #3575

Merged
merged 4 commits into from Oct 23, 2023
Merged

Conversation

azidar
Copy link
Contributor

@azidar azidar commented Oct 13, 2023

Description

This PR introduces two APIs (for discussion) for disabling IO creation.

The first disables IO creation for the rest of the execution of the body of a module. This is in effect a way to separate IO creation from body creation, without forcing a user to declare a bundle/record of the IO's ahead of time.

An example of how this is useful is below. A bunch of trait mix-ins to RawModule define IOs. Then, you can write user code expecting modules of type HasBlahIO, and call associated functions to do stuff. In a way, it enables the scala type of the RawModule contain useful information for connecting to the IOs it declares.

trait HasBlahIO {
  val a, b, c  = IO(Input(Bool()))
  val d, e, f  = IO(Input(Bool()))
  def tieOff(): Unit = {
    // reference IOs declared in trait, callable by concrete class
  }
}
object HasBlahIO {
  def wireUp(params)(m: HasBlahIO): Unit = {
    m.a := ???
     ...
  }
}
trait HasOtherIO {
  val foo, bar = IO(Input(UInt(8.W)))
  val out      = IO(Output(UInt(8.W)))
}


abstract class ExampleInterface extends Module with HasBlahIO with HasOtherIO {
  disableIOCreation()
}

class Example extends ExampleInterface {
  val myReg = RegInit(0.U(8.W))
  out := myReg
  tieOff()
}

// Can also use the type of the module outside an instance
class Parent(child: () => Module) extends RawModule {
  val i = Module(child())
  // Handle HasBlahIO
  i match {
    case h: HasBlahIO =>   HasBlahIO.wireUp(params)(h)
    case _ => 
  }
}

Basically, if you want multi-IO-modules, but also lock down IOs, FixedIORawModule doesn't work well.

The second creates a scope such that any Chisel construction within the scope cannot add an IO. It is a mechanism for the caller to ensure that the callee code does not affect its interface. In general, its a good way to enforce the expected behavior of calling reusable generator code.

Contributor Checklist

  • Did you add at least one test demonstrating the PR?
  • Did you delete any extraneous printlns/debugging code?
  • Did you specify the type of improvement?
  • Did you add appropriate documentation in docs/src?
  • Did you request a desired merge strategy?
  • Did you add text to be included in the Release Notes for this change?

Type of Improvement

  • Feature (or new API)

Desired Merge Strategy

  • Squash: The PR will be squashed and merged (choose this if you have no preference).

Release Notes

Enable users to lock-down the IO-creation of any module by calling disallowIOCreation(). This is useful for building chisel libraries which desire this behavior, but don't want to force a user to declare the entire IO in one bundle.

Reviewer Checklist (only modified by reviewer)

  • Did you add the appropriate labels? (Select the most appropriate one based on the "Type of Improvement")
  • Did you mark the proper milestone (Bug fix: 3.5.x, 3.6.x, or 5.x depending on impact, API modification or big change: 6.0)?
  • Did you review?
  • Did you check whether all relevant Contributor checkboxes have been checked?
  • Did you do one of the following when ready to merge:
    • Squash: You/ the contributor Enable auto-merge (squash), clean up the commit message, and label with Please Merge.
    • Merge: Ensure that contributor has cleaned up their commit history, then merge with Create a merge commit.

@azidar azidar added the Feature New feature, will be included in release notes label Oct 13, 2023
@sequencer
Copy link
Member

this is kinda a location dependent api(while chisel doesn't provide a useful feature to let user permute the execution sequence.) e.g. if this API exist in a trait, user can change the sequence of mixin sequence to workaround, can you describe the use case more detail?
one use case I can think about is the forbid user in diplomacy add InModuleBody IO.
another question is chisel is using IO to declare Property, Probe, is it also forbid here?
another question is if user bore from other modules, is it also forbid here?

@mwachs5
Copy link
Contributor

mwachs5 commented Oct 14, 2023

In module body io creation blocking can be done with existing fixed io and refactoring diplomacy to force people to compute their iOS first... I don't think this is the right way to achieve this for diplomacy. Agree that this could get brittle with trait mixing order... but note scala can also execute code before the existing fixed io class (maybe a deprecated feature?)

@seldridge
Copy link
Member

seldridge commented Oct 16, 2023

but note scala can also execute code before the existing fixed io class (maybe a deprecated feature?)

I thought I made this impossible to pull off, however, I figured out how to do it. trait FixedIOBaseModule is sealed and its only two children, abstract class FixedIORawModule and abstract class FixedIOExtModule, are abstract classes. A user can then only mix-in FixedIORawModule or FixedIOExtModule first. Put differently, as long as there is single inheritance of FixedIOBaseModule then everything is fine. A user can get around this by creating a trait that extends one of the children and using that to achieve multiple inheritance. 🥲 (Maybe there is a way to further lock this down?)

@azidar azidar changed the title Added disalowIOCreation as a public API Added disallowIOCreation as a public API Oct 19, 2023
Copy link
Member

@seldridge seldridge left a comment

Choose a reason for hiding this comment

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

LGTM

This helps pave the way to use of FixedIORawModule, or other evolutions of this idea, that can be used to build stronger interfaces between modules.

I had originally thought that it would be best to start this API out as @deprecate given the fact that it may be somewhat dangerous to rely on. However, we can claw it back if it becomes problematic. Adam has some good points about how if you are already in a MultiIOModule-style pattern that it can only help. I think this makes sense. 👍

core/src/main/scala/chisel3/MultiClock.scala Outdated Show resolved Hide resolved
core/src/main/scala/chisel3/Module.scala Outdated Show resolved Hide resolved
core/src/main/scala/chisel3/Module.scala Show resolved Hide resolved
core/src/main/scala/chisel3/IO.scala Show resolved Hide resolved
@azidar azidar merged commit 7851d9b into main Oct 23, 2023
15 checks passed
@azidar azidar deleted the azidar/open-disallowIOCreation branch October 23, 2023 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature, will be included in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants