chipsalliance / chisel

Chisel: A Modern Hardware Design Language
https://www.chisel-lang.org/
Apache License 2.0
3.93k stars 589 forks source link

Modifying the name of the module that comes from Chisel Library. #1059

Open starbrilliance opened 5 years ago

starbrilliance commented 5 years ago

Type of issue: other enhancement

Impact: API addition (no impact on existing code)

Development Phase: proposal

What is the current behavior?

If I create a Module which from Chisel Library, for example, Queue, there will be a verilog module named Queue. In the meantime, if another person also creates a Queue, my Queue will conflict with his because two modules have the same name. But I can't modify the name of Queue dynamically.

What is the expected behavior?

I want to generate modules of the Chisel Library with custom names. Maybe a better desiredName method will work. However, I can only do it like this:

class MyQueue[T <: Data](gen: T, entries: Int) extends Queue(gen, entries)
val mq = Module(new MyQueue(g, e))

Please tell us about your environment:

What is the use case for changing the behavior? Nothing.

edwardcwang commented 5 years ago

This is only an issue if linking at the Verilog level. If there is another Chisel module named Queue, I believe that Chisel is smart enough to disambiguate them by renaming one of them.

starbrilliance commented 5 years ago

This is only an issue if linking at the Verilog level. If there is another Chisel module named Queue, I believe that Chisel is smart enough to disambiguate them by renaming one of them.

I mean that if I have generated a verilog module named Queue, and my friend has also generated a verilog module named Queue with the same ports except internal logic, then the verilog synthesizer will feel ambiguous when we just simply synthesis our code together. So I think we should have a way to modify the name of library module. Just like a custom module that inherits from Module can modify its verilog name dynamically through the desiredName method.

jackkoenig commented 5 years ago

Chisel 2 provided a -moduleNamePrefix to add a prefix to each Module name. This is one approach to "namespacing" that could solve this issue. For example, you could provide -moduleNamePrefix StarBrilliance then you would have names like StarBrillianceQueue. Does this sound like a satisfactory solution @starbrilliance?

jackkoenig commented 5 years ago

Note that we do not currently support -moduleNamePrefix, I am proposing adding such support to Chisel3/FIRRTL, most likely as a FIRRTL transform.

edwardcwang commented 5 years ago

At the CCC I presented a transform that does more or less this

jackkoenig commented 5 years ago

I think it's a reasonable thing to have in FIRRTL proper if you want to PR it

seldridge commented 5 years ago

Alternatively, if it's a list of keywords to avoid you can use the parent of the Verilog rename transform. (Which would mangle internal signal names, too.)

Edit: RemoveKeywordCollisions

starbrilliance commented 5 years ago

Chisel 2 provided a -moduleNamePrefix to add a prefix to each Module name. This is one approach to "namespacing" that could solve this issue. For example, you could provide -moduleNamePrefix StarBrilliance then you would have names like StarBrillianceQueue. Does this sound like a satisfactory solution @starbrilliance?

Yes. I think this is a useful function.

lsteveol commented 3 years ago

Did this feature ever make it into Chisel3.x or FIRRTL? I've done some searching and I haven't been able to pinpoint a solution.

sequencer commented 3 years ago

Have a try with this pass(may work, not sure.):

class CustomRename(nameMap: Map[String, String]) extends ManipulateNames {
  override def manipulate = (n: String, ns: Namespace) => nameMap.get(n).map(ns.newName)
}

I believe this is the best way to resolve this issue, @starbrilliance and @lsteveol, you can have a try, if it works, we can close this issue :)

lsteveol commented 3 years ago

Thanks! Let me do some leg work on my end. I haven't played around with FIRRTL transforms so I need to look at some examples of how to apply it.

sequencer commented 3 years ago

try to add this to your module(also not tried):

import chisel3.experimental.{annotate, ChiselAnnotation}
annotate(new ChiselAnnotation { def toFirrtl = CustomRename(Map("Queue", "QueueWithBetterName")) })
lsteveol commented 3 years ago

This appears it's on a per Module basis (I'm not 100% sure, so correct me if I'm wrong), I believe what @starbrilliance and myself were wondering is there a way to prefix every module that is generated.

For instance, extending this example I have a Chisel based IP that has various configs. The overall structure of the design looks like this from a class <mod> extends [LazyModule|Module] standpoint.

MyIP
  MyController
  Queue
  TLFragmenter
  MySubblock

My chiptop guys think Chisel is weird (because they are short sighted :)) so they want me to produce the verilog for each instance. Being that there are different configs I will have something like MyIPConfig1 and MyIPConfig2. They have similar hierarchies and therefore they instantiate Queue, TLFragmenter, and various other modules.

Ideally I could use a transform similar to what you have but it would be nice to be able to do something like

//configVars is maybe some string I set at the top level
annotate(new ChiselAnnotation { def toFirrtl = CustomPrefix(s"MyIP_$configVars") })

Which would change the module names to something like the following for each generation (I run a separate ChiselStage.emitVerilog in this case)

MyIPConfg1
  MyIPConfg1_MyController
  MyIPConfg1_Queue
  MyIPConfg1_TLFragmenter
  MyIPConfg1_MySubblock
MyIPConfg2
  MyIPConfg2_MyController
  MyIPConfg2_Queue
  MyIPConfg2_TLFragmenter
  MyIPConfg2_MySubblock

This would uniquify every module in the hierarchy, mainly by just prefixing each module generated. So it's similar to what you've provided, but almost a global level module naming, and I don't have to explicitly name each Module, especially if I re-use Modules across various designs.

I actually have a Perl script that uses Verilog-Perl to do this, but it would be nice to have it come directly from the Chisel generated Verilog.

sequencer commented 3 years ago

I see, this has been done in rocket-chip with Aspect. Take a look at freechips.rocketchip.aspects.RenameModulesAspect

lsteveol commented 3 years ago

Ah thanks for the tip! I will check that out. Probably what I need.

hukangha commented 3 years ago

Hi, @lsteveol have you ever find a solution? I'm with chisel3.4.1 and there does not seem to have any "FIRRTL Transform Options" to do the global prefix yet.

lsteveol commented 3 years ago

Hi @hukangha I haven't tried anything out. I have a custom perl script which allows me to prefix modules so I've just been using that for now. I plan to look into this in the next few months when time allows.

hukangha commented 3 years ago

@lsteveol Yes, seems script with regex is a temporary solution for now.

Some thoughts: I think namespacing or global prefixing is an attractive feature for those team that might want to try chisel and not so determined switching yet. It is different from overriding methods like RenameModulesAspect because it enable someone to connect generated designs in verilog context. And overriding does not go into chisel library without manually extending the library module.

zhutmost commented 3 years ago

Hi, do we have a solution now? (excepts regex)

I am generating two designs (one for FPGA and the other for ASIC) with some common codes. and I have to run some Verilog-level simulation with these two designs together, but VCS reports that some modules in them have the same name. I have no idea how to solve the naming conflict in the Chisel-level.

ekiwi commented 3 years ago

Would this work for you? https://scastie.scala-lang.org/D6cf9rZySiiX7GJC3XWzrQ

import chisel3._
import chisel3.stage.ChiselStage

import firrtl._
import firrtl.options.Dependency
import firrtl.transforms.DedupModules
import firrtl.annotations.NoTargetAnnotation
import firrtl.passes.PassException
import firrtl.stage.RunFirrtlTransformAnnotation

////////////////
// Pass
////////////////

/** Specifies a global prefix for all module names. */
case class ModulePrefix(prefix: String) extends NoTargetAnnotation

object PrefixModulesPass extends Transform with DependencyAPIMigration {
  // we run after deduplication to save some work
  override def prerequisites = Seq(Dependency[DedupModules])

  // we do not invalidate the results of any prior passes
  override def invalidates(a: Transform) = false

  override protected def execute(state: CircuitState): CircuitState = {
    val prefixes = state.annotations.collect{ case a: ModulePrefix => a.prefix }.distinct
    prefixes match {
      case Seq() =>
        logger.info("[PrefixModulesPass] No ModulePrefix annotation found.")
        state
      case Seq("") => state
      case Seq(prefix) =>
        val c = state.circuit.mapModule(onModule(_, prefix))
        state.copy(circuit = c.copy(main = prefix + c.main))
      case other =>
        throw new PassException(s"[PrefixModulesPass] found more than one prefix annotation: $other")
    }
  }

  private def onModule(m: ir.DefModule, prefix: String): ir.DefModule = m match {
    case e : ir.ExtModule => e.copy(name = prefix + e.name)
    case mod: ir.Module =>
      val name = prefix + mod.name
      val body = onStmt(mod.body, prefix)
      mod.copy(name=name, body=body)
  }

  private def onStmt(s: ir.Statement, prefix: String): ir.Statement = s match {
    case i : ir.DefInstance => i.copy(module = prefix + i.module)
    case other => other.mapStmt(onStmt(_, prefix))
  }
}

////////////////
// Circuit
////////////////

class Child extends MultiIOModule {
  val in = IO(Input(Bool()))
  val out = IO(Output(Bool()))
  out := ~in
}

class Foo extends MultiIOModule {
  val in = IO(Input(Bool()))
  val out = IO(Output(Bool()))

  val c0 = Module(new Child)
  val c1 = Module(new Child)

  c0.in := in
  c1.in := c0.out
  out := c1.out
}

val prefix = "Test_"
val annos = Seq(RunFirrtlTransformAnnotation(Dependency(PrefixModulesPass)), ModulePrefix(prefix))

println((new ChiselStage).emitVerilog(new Foo, annotations=annos))
zhutmost commented 3 years ago

@ekiwi Cool. Thanks a lot.

kenzhang82 commented 3 years ago

Any progress on upstreaming the above changes as a FIRRTL pass?

ekiwi commented 3 years ago

Any progress on upstreaming the above changes as a FIRRTL pass?

No one is currently working on that. If you want to you can start a PR. You can base it on the code that I posted and then add some tests, documentation and some more thoughts on what the exact API should be.

kenzhang82 commented 3 years ago

Thanks @ekiwi, I should find some time soon to look into this, thanks for sharing.

kenzhang82 commented 3 years ago

I am trying to implement and test this at the moment, my code looks like the following, note that I am doing it from firrtl repo, so probably worth of creating another issue in there:

package firrtl.passes

import firrtl._
import firrtl.options.{Dependency, RegisteredTransform, ShellOption}
import firrtl.transforms.DedupModules
import firrtl.annotations.NoTargetAnnotation
import firrtl.stage.RunFirrtlTransformAnnotation

////////////////
// Pass
////////////////

/** Specifies a global prefix for all module names. */
case class ModulePrefix(prefix: String = "abcdefg") extends NoTargetAnnotation

class PrefixModulesPass extends Transform with DependencyAPIMigration with RegisteredTransform {
  // we run after deduplication to save some work
  override def prerequisites = Seq(Dependency[DedupModules])

  // we do not invalidate the results of any prior passes
  override def invalidates(a: Transform) = false

  val options = Seq(
    new ShellOption[String](
      longOption = "module-name-prefix",
      toAnnotationSeq = (a: String) =>
        Seq(ModulePrefix("abcdefg"), RunFirrtlTransformAnnotation(new PrefixModulesPass)),
      helpText = "Global prefix to every modules",
      shortOption = Some("prefix"),
      helpValueName = Some("<prefix>")
    )
  )

  override protected def execute(state: CircuitState): CircuitState = {
    val prefixes = state.annotations.collect{ case a: ModulePrefix => a.prefix }.distinct
    prefixes match {
      case Seq() =>
        logger.info("[PrefixModulesPass] No ModulePrefix annotation found.")
        state
      case Seq("") => state
      case Seq(prefix) =>
        val c = state.circuit.mapModule(onModule(_, prefix))
        state.copy(circuit = c.copy(main = prefix + c.main))
      case other =>
        throw new PassException(s"[PrefixModulesPass] found more than one prefix annotation: $other")
    }
  }

  private def onModule(m: ir.DefModule, prefix: String): ir.DefModule = m match {
    case e : ir.ExtModule => e.copy(name = prefix + e.name)
    case mod: ir.Module =>
      val name = prefix + mod.name
      val body = onStmt(mod.body, prefix)
      mod.copy(name=name, body=body)
  }

  private def onStmt(s: ir.Statement, prefix: String): ir.Statement = s match {
    case i : ir.DefInstance => i.copy(module = prefix + i.module)
    case other => other.mapStmt(onStmt(_, prefix))
  }
}

Then I register this transformation by appending firrtl.passes.PrefixModulesPass inside src/main/resources/META-INF/firrtl.options.RegisteredTransform.

Then I run sbt assembly to compile everything, this compiled without any problem, I can see this CLI option when I do:

./utils/bin/firrtl --help
FIRRTL Transform Options
  --no-dce                 Disable dead code elimination
  --no-check-comb-loops    Disable combinational loop checking
  -fil, --inline <circuit>[.<module>[.<instance>]][,...]
                           Inline selected modules
  -clks, --list-clocks -c:<circuit>:-m:<module>:-o:<filename>
                           List which signal drives each clock of every descendent of specified modules
  --no-asa                 Disable assert submodule assumptions
  --no-constant-propagation
                           Disable constant propagation elimination
  -prefix, --module-name-prefix <prefix>
                           Global prefix to every modules

However, when I run an example circuit to perform the transformation:

./utils/bin/firrtl -td regress -i regress/RocketCore.fir --module-name-prefix abcdefg

The output Verilog doesn't have any of the prefixes I specify. Note that I am hardcoding the prefix to start with, it would be great to somehow pass this from the command line. Any idea what is going wrong here? @seldridge @ekiwi @jackkoenig

sequencer commented 3 years ago

You can submit a PR to FIRRTL, then we can help you debugging your issue;)

kenzhang82 commented 3 years ago

Thanks @sequencer, I will set it up now.

kenzhang82 commented 3 years ago

@sequencer here you go, initial PR is up (but not working yet): https://github.com/chipsalliance/firrtl/pull/2200

It would be excellent if you can help out here, I am a newbie in writing FIRRTL transformation, so please bear with me... :-)

sequencer commented 3 years ago

Sure I’ll take a look later this night.

kenzhang82 commented 3 years ago

In case anyone is still wondering if there is a solution, here is transform that Chipyard folks are using for tapeout https://github.com/ucb-bar/barstools/blob/master/tapeout/src/main/scala/barstools/tapeout/transforms/AddSuffixToModuleNames.scala

I think we should ask Chipyard folks to upstream this. Thoughts? @jackkoenig

jiegec commented 2 years ago

ping, any progress?

hhhhhyang commented 2 years ago

I've tried @ekiwi 's solution to add a suffix string to module name in generated verilog. Quite elegent and helpful. But if you use dontTouch in chisel code, the firrtl.transform.DontTouchAnnotation will throw exceptions "DontTouchNotFoundException". For now, I just simply remove the dontTouch out of my code, to get round of this.

ekiwi commented 2 years ago

But if you use dontTouch in chisel code, the firrtl.transform.DontTouchAnnotation will throw exceptions "DontTouchNotFoundException". For now, I just simply remove the dontTouch out of my code, to get round of this.

Yeah, so annotations do not work with the hacky prototype that I posted. See: https://github.com/chipsalliance/firrtl/pull/2200/files#r622276537

Tang-Haojin commented 1 year ago

Will there be some workaround available after chisel 3.6.0? It seems that the use of firrtl package is deprecated, and I have not found something similar in circt package.

Would this work for you? https://scastie.scala-lang.org/D6cf9rZySiiX7GJC3XWzrQ

import chisel3._
import chisel3.stage.ChiselStage

import firrtl._
import firrtl.options.Dependency
import firrtl.transforms.DedupModules
import firrtl.annotations.NoTargetAnnotation
import firrtl.passes.PassException
import firrtl.stage.RunFirrtlTransformAnnotation

////////////////
// Pass
////////////////

/** Specifies a global prefix for all module names. */
case class ModulePrefix(prefix: String) extends NoTargetAnnotation

object PrefixModulesPass extends Transform with DependencyAPIMigration {
  // we run after deduplication to save some work
  override def prerequisites = Seq(Dependency[DedupModules])

  // we do not invalidate the results of any prior passes
  override def invalidates(a: Transform) = false

  override protected def execute(state: CircuitState): CircuitState = {
    val prefixes = state.annotations.collect{ case a: ModulePrefix => a.prefix }.distinct
    prefixes match {
      case Seq() =>
        logger.info("[PrefixModulesPass] No ModulePrefix annotation found.")
        state
      case Seq("") => state
      case Seq(prefix) =>
        val c = state.circuit.mapModule(onModule(_, prefix))
        state.copy(circuit = c.copy(main = prefix + c.main))
      case other =>
        throw new PassException(s"[PrefixModulesPass] found more than one prefix annotation: $other")
    }
  }

  private def onModule(m: ir.DefModule, prefix: String): ir.DefModule = m match {
    case e : ir.ExtModule => e.copy(name = prefix + e.name)
    case mod: ir.Module =>
      val name = prefix + mod.name
      val body = onStmt(mod.body, prefix)
      mod.copy(name=name, body=body)
  }

  private def onStmt(s: ir.Statement, prefix: String): ir.Statement = s match {
    case i : ir.DefInstance => i.copy(module = prefix + i.module)
    case other => other.mapStmt(onStmt(_, prefix))
  }
}

////////////////
// Circuit
////////////////

class Child extends MultiIOModule {
  val in = IO(Input(Bool()))
  val out = IO(Output(Bool()))
  out := ~in
}

class Foo extends MultiIOModule {
  val in = IO(Input(Bool()))
  val out = IO(Output(Bool()))

  val c0 = Module(new Child)
  val c1 = Module(new Child)

  c0.in := in
  c1.in := c0.out
  out := c1.out
}

val prefix = "Test_"
val annos = Seq(RunFirrtlTransformAnnotation(Dependency(PrefixModulesPass)), ModulePrefix(prefix))

println((new ChiselStage).emitVerilog(new Foo, annotations=annos))
seldridge commented 1 year ago

If you are using CIRCT, you can use NestedPrefixModuleAnnotation: https://circt.llvm.org/docs/Dialects/FIRRTL/FIRRTLAnnotations/#nestedprefixmodulesannotation

If you manually construct an annotation like this that is targeted at the top-module of the circuit, it will rename like expected.

Example:

circuit Top : %[[
  {
    "class":"sifive.enterprise.firrtl.NestedPrefixModulesAnnotation",
    "target":"~Top|Top",
    "prefix":"Prefix_",
    "inclusive":false
  },
  {
    "class": "firrtl.transforms.DontTouchAnnotation",
    "target": "~Top|Foo>in"
  }
]]
  module Foo :
    input in: UInt<8>

  module Top :
    input in : UInt<8>

    inst foo of Foo
    foo.in <= in

Compiling with firtool Top.fir produces:

// Generated by CIRCT unknown git version
module Prefix_Foo(
  input [7:0] in);

endmodule

module Top(
  input [7:0] in);

  Prefix_Foo foo (
    .in (in)
  );
endmodule

There is no publicly available annotation with this format, but one could be either manually constructed or hacked together to use this API directly from Chisel. This API may rapidly change in the future as we move towards doing this type of prefixing directly in Chisel or explore other alternatives to avoid the "Queue name problem".

Tang-Haojin commented 1 year ago

Thank you!


发件人: Schuyler Eldridge @.> 发送时间: Sunday, January 29, 2023 6:34:58 AM 收件人: chipsalliance/chisel3 @.> 抄送: Tang Haojin @.>; Comment @.> 主题: Re: [chipsalliance/chisel3] Modifying the name of the module that comes from Chisel Library. (#1059)

If you are using CIRCT, you can use NestedPrefixModuleAnnotation: https://circt.llvm.org/docs/Dialects/FIRRTL/FIRRTLAnnotations/#nestedprefixmodulesannotation

If you manually construct an annotation like this that is targeted at the top-module of the circuit, it will rename like expected.

Example:

circuit Top : %[[ { "class":"sifive.enterprise.firrtl.NestedPrefixModulesAnnotation", "target":"~Top|Top", "prefix":"Prefix_", "inclusive":false }, { "class": "firrtl.transforms.DontTouchAnnotation", "target": "~Top|Foo>in" } ]] module Foo : input in: UInt<8>

module Top : input in : UInt<8>

inst foo of Foo
foo.in <= in

Compiling with firtool Top.fir produces:

// Generated by CIRCT unknown git version module Prefix_Foo( input [7:0] in);

endmodule

module Top( input [7:0] in);

Prefix_Foo foo ( .in (in) ); endmodule

There is no publicly available annotation with this format, but one could be either manually constructed or hacked together to use this API directly from Chisel. This API may rapidly change in the future as we move towards doing this type of prefixing directly in Chisel or explore other alternatives to avoid the "Queue name problem".

― Reply to this email directly, view it on GitHubhttps://github.com/chipsalliance/chisel3/issues/1059#issuecomment-1407502593, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AK7UKCCNMC5W7X53EZ6Z4FLWUWNJFANCNFSM4HDRZ5VA. You are receiving this because you commented.Message ID: @.***>

jiegec commented 1 year ago

Generating the new annotation for MLIR FIRRTL Compiler (Inspired by @sequencer ):

package sifive {
  package enterprise {
    package firrtl {
      import _root_.firrtl.annotations._

      case class NestedPrefixModulesAnnotation(
          val target: Target,
          prefix: String,
          inclusive: Boolean
      ) extends SingleTargetAnnotation[Target] {

        def duplicate(n: Target): Annotation =
          NestedPrefixModulesAnnotation(target, prefix, inclusive)
      }
    }

  }

}

object AddPrefix {
  def apply(module: Module, prefix: String, inclusive: Boolean = true) = {
      annotate(new ChiselAnnotation {
        def toFirrtl =
          new NestedPrefixModulesAnnotation(module.toTarget, prefix, inclusive)
      })
  }
}
sashimi-yzh commented 1 year ago

It works! @jiegec

But there is a waring.

Importing from firrtl is deprecated as of Chisel's 3.6.0 release.
[warn]       import _root_.firrtl.annotations._
[warn]              ^