Skip to content

Commit

Permalink
[codegen] Emit literal identifiers for numeric ids
Browse files Browse the repository at this point in the history 
Change the FIRRTL serializer to use literal identifiers if any identifiers
begin with a leading digit.  This is a FIRRTL 3.0.0 feature that
simplifies parsing.

This enables literal identifier emission anywhere.  However, this code
path is only reachable via MixedVec due to mangling of numeric names to
add a leading underscore in all other circumstances.  In the future, this
commit will enable removing this restriction and switching to literal
identifiers when this happens.

No changes are made to annotation targets.  E.g., the literal identifier,
"wire `42`" in circuit "Foo" and module "Bar" is still referred to by a
local target "~Foo|Bar>42".  This is intentional (for now).  Targets don't
have parsing ambiguity like FIRRTL text and the name of this wire is still
"42" not "`42`".  It follows that the storage of this name in FIRRTL IR is
not changed.  This is the same way that this is handled in CIRCT.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
  • Loading branch information
@seldridge
seldridge committed Jun 21, 2023
1 parent b0a8977 commit d9de057
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 19 deletions.
50 changes: 31 additions & 19 deletions firrtl/src/main/scala/firrtl/ir/Serializer.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,25 @@ object Serializer {
case other => b ++= other.serialize // Handle user-defined nodes
}

/** Hash map containing names that were changed due to legalization. */
private val legalizedNames = scala.collection.mutable.HashMap.empty[String, String]

/** Generate a legal FIRRTL name. */
private def legalize(name: String): String = name match {
// If the name starts with a digit, then escape it with backticks.
case _ if name.head.isDigit => legalizedNames.getOrElseUpdate(name, s"`$name`")
case _ => name
}

private def s(str: StringLit)(implicit b: StringBuilder, indent: Int): Unit = b ++= str.serialize

private def s(node: Expression)(implicit b: StringBuilder, indent: Int): Unit = node match {
case Reference(name, _) => b ++= name
case Reference(name, _) => b ++= legalize(name)
case DoPrim(op, args, consts, _) =>
b ++= op.toString; b += '('; s(args, ", ", consts.isEmpty); s(consts, ", "); b += ')'
case UIntLiteral(value, width) =>
b ++= "UInt"; s(width); b ++= "(0h"; b ++= value.toString(16); b ++= ")"
case SubField(expr, name, _) => s(expr); b += '.'; b ++= name
case SubField(expr, name, _) => s(expr); b += '.'; b ++= legalize(name)
case SubIndex(expr, value, _) => s(expr); b += '['; b ++= value.toString; b += ']'
case SubAccess(expr, index, _) => s(expr); b += '['; s(index); b += ']'
case Mux(cond, tval, fval, _) =>
Expand Down Expand Up @@ -227,7 +237,7 @@ object Serializer {
}

private def s(node: Statement)(implicit b: StringBuilder, indent: Int): Unit = node match {
case DefNode(info, name, value) => b ++= "node "; b ++= name; b ++= " = "; s(value); s(info)
case DefNode(info, name, value) => b ++= "node "; b ++= legalize(name); b ++= " = "; s(value); s(info)
case Connect(info, loc, expr) => b ++= "connect "; s(loc); b ++= ", "; s(expr); s(info)
case c: Conditionally => b ++= sIt(c).mkString
case EmptyStmt => b ++= "skip"
Expand All @@ -240,13 +250,15 @@ object Serializer {
if (args.nonEmpty) b ++= ", "; s(args, ", "); b += ')'
sStmtName(print.name); s(info)
case IsInvalid(info, expr) => b ++= "invalidate "; s(expr); s(info)
case DefWire(info, name, tpe) => b ++= "wire "; b ++= name; b ++= " : "; s(tpe); s(info)
case DefWire(info, name, tpe) => b ++= "wire "; b ++= legalize(name); b ++= " : "; s(tpe); s(info)
case DefRegister(info, name, tpe, clock) =>
b ++= "reg "; b ++= name; b ++= " : "; s(tpe); b ++= ", "; s(clock); s(info)
b ++= "reg "; b ++= legalize(name); b ++= " : "; s(tpe); b ++= ", "; s(clock); s(info)
case DefRegisterWithReset(info, name, tpe, clock, reset, init) =>
b ++= "regreset "; b ++= name; b ++= " : "; s(tpe); b ++= ", "; s(clock); b ++= ", "; s(reset); b ++= ", ";
b ++= "regreset "; b ++= legalize(name); b ++= " : "; s(tpe); b ++= ", "; s(clock); b ++= ", "; s(reset);
b ++= ", ";
s(init); s(info)
case DefInstance(info, name, module, _) => b ++= "inst "; b ++= name; b ++= " of "; b ++= module; s(info)
case DefInstance(info, name, module, _) =>
b ++= "inst "; b ++= legalize(name); b ++= " of "; b ++= legalize(module); s(info)
case DefMemory(
info,
name,
Expand All @@ -259,14 +271,14 @@ object Serializer {
readwriters,
readUnderWrite
) =>
b ++= "mem "; b ++= name; b ++= " :"; s(info); newLineAndIndent(1)
b ++= "mem "; b ++= legalize(name); b ++= " :"; s(info); newLineAndIndent(1)
b ++= "data-type => "; s(dataType); newLineAndIndent(1)
b ++= "depth => "; b ++= depth.toString(); newLineAndIndent(1)
b ++= "read-latency => "; b ++= readLatency.toString; newLineAndIndent(1)
b ++= "write-latency => "; b ++= writeLatency.toString; newLineAndIndent(1)
readers.foreach { r => b ++= "reader => "; b ++= r; newLineAndIndent(1) }
writers.foreach { w => b ++= "writer => "; b ++= w; newLineAndIndent(1) }
readwriters.foreach { r => b ++= "readwriter => "; b ++= r; newLineAndIndent(1) }
readers.foreach { r => b ++= "reader => "; b ++= legalize(r); newLineAndIndent(1) }
writers.foreach { w => b ++= "writer => "; b ++= legalize(w); newLineAndIndent(1) }
readwriters.foreach { r => b ++= "readwriter => "; b ++= legalize(r); newLineAndIndent(1) }
b ++= "read-under-write => "; b ++= readUnderWrite.toString
case Attach(info, exprs) =>
// exprs should never be empty since the attach statement takes *at least* two signals according to the spec
Expand All @@ -278,13 +290,13 @@ object Serializer {
// WIR
case firrtl.CDefMemory(info, name, tpe, size, seq, readUnderWrite) =>
if (seq) b ++= "smem " else b ++= "cmem "
b ++= name; b ++= " : "; s(tpe); b ++= " ["; b ++= size.toString(); b += ']'
b ++= legalize(name); b ++= " : "; s(tpe); b ++= " ["; b ++= size.toString(); b += ']'
if (readUnderWrite != ReadUnderWrite.Undefined) { // undefined is the default
b += ' '; b ++= readUnderWrite.toString
}
s(info)
case firrtl.CDefMPort(info, name, _, mem, exps, direction) =>
b ++= direction.serialize; b ++= " mport "; b ++= name; b ++= " = "; b ++= mem
b ++= direction.serialize; b ++= " mport "; b ++= legalize(name); b ++= " = "; b ++= legalize(mem)
b += '['; s(exps.head); b ++= "], "; s(exps(1)); s(info)
case ProbeDefine(info, sink, probeExpr) =>
b ++= "define "; s(sink); b ++= " = "; s(probeExpr); s(info)
Expand Down Expand Up @@ -322,7 +334,7 @@ object Serializer {
}

private def s(node: Field)(implicit b: StringBuilder, indent: Int): Unit = node match {
case Field(name, flip, tpe) => s(flip); b ++= name; b ++= " : "; s(tpe)
case Field(name, flip, tpe) => s(flip); b ++= legalize(name); b ++= " : "; s(tpe)
}

private def s(node: Type)(implicit b: StringBuilder, indent: Int): Unit = node match {
Expand Down Expand Up @@ -350,7 +362,7 @@ object Serializer {

private def s(node: Port)(implicit b: StringBuilder, indent: Int): Unit = node match {
case Port(info, name, direction, tpe) =>
s(direction); b += ' '; b ++= name; b ++= " : "; s(tpe); s(info)
s(direction); b += ' '; b ++= legalize(name); b ++= " : "; s(tpe); s(info)
}

private def s(node: Param)(implicit b: StringBuilder, indent: Int): Unit = node match {
Expand All @@ -367,7 +379,7 @@ object Serializer {
case Module(info, name, ports, body) =>
val start = {
implicit val b = new StringBuilder
doIndent(0); b ++= "module "; b ++= name; b ++= " :"; s(info)
doIndent(0); b ++= "module "; b ++= legalize(name); b ++= " :"; s(info)
ports.foreach { p => newLineAndIndent(1); s(p) }
newLineNoIndent() // add a blank line between port declaration and body
newLineNoIndent() // newline for body, sIt will indent
Expand All @@ -376,14 +388,14 @@ object Serializer {
Iterator(start) ++ sIt(body)(indent + 1)
case ExtModule(info, name, ports, defname, params) =>
implicit val b = new StringBuilder
doIndent(0); b ++= "extmodule "; b ++= name; b ++= " :"; s(info)
doIndent(0); b ++= "extmodule "; b ++= legalize(name); b ++= " :"; s(info)
ports.foreach { p => newLineAndIndent(1); s(p) }
newLineAndIndent(1); b ++= "defname = "; b ++= defname
params.foreach { p => newLineAndIndent(1); s(p) }
Iterator(b.toString)
case IntModule(info, name, ports, intrinsic, params) =>
implicit val b = new StringBuilder
doIndent(0); b ++= "intmodule "; b ++= name; b ++= " :"; s(info)
doIndent(0); b ++= "intmodule "; b ++= legalize(name); b ++= " :"; s(info)
ports.foreach { p => newLineAndIndent(1); s(p) }
newLineAndIndent(1); b ++= "intrinsic = "; b ++= intrinsic
params.foreach { p => newLineAndIndent(1); s(p) }
Expand All @@ -401,7 +413,7 @@ object Serializer {
val prelude = {
implicit val b = new StringBuilder
b ++= s"FIRRTL version ${version.serialize}\n"
b ++= "circuit "; b ++= circuit.main; b ++= " :";
b ++= "circuit "; b ++= legalize(circuit.main); b ++= " :";
if (annotations.nonEmpty) {
b ++= "%["; b ++= JsonProtocol.serialize(annotations); b ++= "]";
}
Expand Down
76 changes: 76 additions & 0 deletions firrtl/src/test/scala/firrtlTests/SerializerSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,4 +282,80 @@ class SerializerSpec extends AnyFlatSpec with Matchers {
"Once we traverse the serializer, everything should execute"
)
}

it should "add backticks to names which begin with a numeric character" in {
info("circuit okay!")
Serializer.serialize(Circuit(NoInfo, Seq.empty[DefModule], "42_Circuit")) should include("circuit `42_Circuit`")

info("modules okay!")
Serializer.serialize(Module(NoInfo, "42_module", Seq.empty, Block(Seq.empty))) should include("module `42_module`")
// TODO: an external module with a numeric defname should probably be rejected
Serializer.serialize(ExtModule(NoInfo, "42_extmodule", Seq.empty, "<TODO>", Seq.empty)) should include(
"extmodule `42_extmodule`"
)
Serializer.serialize(IntModule(NoInfo, "42_intmodule", Seq.empty, "foo", Seq.empty)) should include(
"intmodule `42_intmodule`"
)

info("ports okay!")
Serializer.serialize(Port(NoInfo, "42_port", Input, UIntType(IntWidth(1)))) should include("input `42_port`")

info("types okay!")
Serializer.serialize(BundleType(Seq(Field("42_field", Default, UIntType(IntWidth(1)))))) should include(
"{ `42_field` : UInt<1>}"
)

info("declarations okay!")
Serializer.serialize(DefNode(NoInfo, "42_dest", Reference("42_src"))) should include("node `42_dest` = `42_src`")
Serializer.serialize(DefWire(NoInfo, "42_wire", UIntType(IntWidth(1)))) should include("wire `42_wire`")
Serializer.serialize(DefRegister(NoInfo, "42_reg", UIntType(IntWidth(1)), Reference("42_clock"))) should include(
"reg `42_reg` : UInt<1>, `42_clock`"
)
Serializer.serialize(
DefRegisterWithReset(
NoInfo,
"42_regreset",
UIntType(IntWidth(1)),
Reference("42_clock"),
Reference("42_reset"),
Reference("42_init")
)
) should include("regreset `42_regreset` : UInt<1>, `42_clock`, `42_reset`, `42_init`")
Serializer.serialize(DefInstance(NoInfo, "42_inst", "42_module")) should include("inst `42_inst` of `42_module`")
(Serializer
.serialize(
DefMemory(
NoInfo,
"42_mem",
UIntType(IntWidth(1)),
8,
1,
1,
Seq("42_r"),
Seq("42_w"),
Seq("42_rw"),
ReadUnderWrite.Undefined
)
)
.split('\n')
.map(_.strip) should contain).allOf(
"mem `42_mem` :",
"reader => `42_r`",
"writer => `42_w`",
"readwriter => `42_rw`"
)
Serializer.serialize(
CDefMemory(NoInfo, "42_cmem", UIntType(IntWidth(1)), 8, true, ReadUnderWrite.Undefined)
) should include("smem `42_cmem`")
Serializer.serialize(
firrtl.CDefMPort(
NoInfo,
"42_memport",
UIntType(IntWidth(1)),
"42_mem",
Seq(UIntLiteral(0, IntWidth(1)), Reference("42_clock")),
firrtl.MRead
)
) should include("mport `42_memport` = `42_mem`[UInt<1>(0h0)], `42_clock`")
}
}

0 comments on commit d9de057

Please sign in to comment.