-
Notifications
You must be signed in to change notification settings - Fork 643
Create method for marking all ports of a Module with DontTouchAnnotation
#3866
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,10 +2,11 @@ | |||||
|
|
||||||
| package chisel3 | ||||||
|
|
||||||
| import chisel3.experimental.{annotate, requireIsHardware, ChiselAnnotation} | ||||||
| import chisel3.experimental.{annotate, requireIsHardware, BaseModule, ChiselAnnotation, ChiselMultiAnnotation} | ||||||
| import chisel3.properties.Property | ||||||
| import chisel3.reflect.DataMirror | ||||||
| import firrtl.transforms.DontTouchAnnotation | ||||||
| import firrtl.annotations.Annotation | ||||||
|
|
||||||
| /** Marks that a signal's leaves are an optimization barrier to Chisel and the | ||||||
| * FIRRTL compiler. This has the effect of guaranteeing that a signal will not | ||||||
|
|
@@ -46,4 +47,18 @@ object dontTouch { | |||||
| data | ||||||
| } | ||||||
|
|
||||||
| /** Mark a module so that its ports will not get optimized away. | ||||||
| * | ||||||
| * @param module whose ports shouldl not get optimized away | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| */ | ||||||
| def modulePorts(module: BaseModule): BaseModule = { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You need an implicit SourceInfo here for |
||||||
|
|
||||||
| annotate(new ChiselMultiAnnotation { | ||||||
| def toFirrtl: Seq[Annotation] = (DataMirror.fullModulePorts(module).map { | ||||||
| case (_, data) => DontTouchAnnotation(data.toNamed) | ||||||
| }) | ||||||
|
Comment on lines
+57
to
+59
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
| }) | ||||||
| module | ||||||
| } | ||||||
|
|
||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -123,4 +123,66 @@ class DontTouchSpec extends ChiselFlatSpec with Utils { | |
| // Ensure can compile the result. | ||
| compile(new HasProbesAndProperties()) | ||
| } | ||
|
|
||
| "dontTouch.modulePorts" should "mark all ports of a module" in { | ||
| class ModuleWithPorts extends Module { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we add a test for a Module with property and/or Probe ports? Im unclear if those are expected to be annotated, or there is supposed to be an error, or...? |
||
| val io = IO(new Bundle { | ||
| val a = Output(UInt(32.W)) | ||
| val b = Input(new Bundle { | ||
| val x = Bool() | ||
| val y = new Bundle { | ||
| val foo = UInt(4.W) | ||
| } | ||
| }) | ||
| }) | ||
| io.a := DontCare | ||
| } | ||
| class DummyWrapper extends Module { | ||
| val dut = Module(new ModuleWithPorts) | ||
| dontTouch.modulePorts(dut) | ||
| } | ||
|
|
||
| val (_, annos) = getFirrtlAndAnnos(new DummyWrapper) | ||
| (annos should contain).allOf( | ||
| DontTouchAnnotation("~DummyWrapper|ModuleWithPorts>clock".rt), | ||
| DontTouchAnnotation("~DummyWrapper|ModuleWithPorts>reset".rt), | ||
| DontTouchAnnotation("~DummyWrapper|ModuleWithPorts>io".rt), | ||
| DontTouchAnnotation("~DummyWrapper|ModuleWithPorts>io.b".rt), | ||
| DontTouchAnnotation("~DummyWrapper|ModuleWithPorts>io.b.y".rt), | ||
| DontTouchAnnotation("~DummyWrapper|ModuleWithPorts>io.b.y.foo".rt), | ||
| DontTouchAnnotation("~DummyWrapper|ModuleWithPorts>io.b.x".rt), | ||
| DontTouchAnnotation("~DummyWrapper|ModuleWithPorts>io.a".rt) | ||
| ) | ||
| } | ||
|
|
||
| "dontTouch.modulePorts" should "mark bored ports" in { | ||
| class ModuleToBore extends Module { | ||
| val io = IO(new Bundle { | ||
| val a = Output(UInt(32.W)) | ||
| val b = Input(Bool()) | ||
| }) | ||
| io.a := DontCare | ||
|
|
||
| val boreMe = Wire(UInt(4.W)) | ||
| boreMe := DontCare | ||
| } | ||
| class BoreModule extends Module { | ||
| val boreOut = IO(Output(UInt(4.W))) | ||
| val dut = Module(new ModuleToBore) | ||
|
|
||
| boreOut := chisel3.util.experimental.BoringUtils.bore(dut.boreMe) | ||
| dontTouch.modulePorts(dut) | ||
| } | ||
|
|
||
| val (_, annos) = getFirrtlAndAnnos(new BoreModule) | ||
| (annos should contain).allOf( | ||
| DontTouchAnnotation("~BoreModule|ModuleToBore>clock".rt), | ||
| DontTouchAnnotation("~BoreModule|ModuleToBore>reset".rt), | ||
| DontTouchAnnotation("~BoreModule|ModuleToBore>io".rt), | ||
| DontTouchAnnotation("~BoreModule|ModuleToBore>io.b".rt), | ||
| DontTouchAnnotation("~BoreModule|ModuleToBore>io.a".rt), | ||
| DontTouchAnnotation("~BoreModule|BoreModule>dut.boreOut_bore".rt) | ||
| ) | ||
| } | ||
|
|
||
| } | ||
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.
does this already do the right thing w.r.t. property ports and probe ports?
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.
As we discussed in dev, we're fine to add this API but we should direct users to
PublicandProbehere in the API docs depending on the use case.