Skip to content
This repository has been archived by the owner on Aug 20, 2024. It is now read-only.

Commit

Permalink
Make MemConf's MemPort serialization deterministic (#2508)
Browse files Browse the repository at this point in the history
Problem: MemConf serialization of MemPorts was not deterministic
and the ordering seems to have changed as we move projects to 2.13
Downstream project can be adversely affected by changes in ordering
This changes specifies as specific ordering that should be compatible with
the historical one.
  • Loading branch information
chick authored Apr 8, 2022
1 parent aea60aa commit f7b4aa8
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 2 deletions.
19 changes: 17 additions & 2 deletions src/main/scala/firrtl/passes/memlib/MemConf.scala
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,21 @@ case object MaskedReadWritePort extends MemPort("mrw")

object MemPort {

val all = Set(ReadPort, WritePort, MaskedWritePort, ReadWritePort, MaskedReadWritePort)
// This is the order that ports will render in MemConf.portsStr
val ordered: Seq[MemPort] = Seq(
MaskedReadWritePort,
MaskedWritePort,
ReadWritePort,
WritePort,
ReadPort
)

val all: Set[MemPort] = ordered.toSet
// uses orderedPorts when sorting MemPorts
implicit def ordering: Ordering[MemPort] = {
val orderedPorts = ordered.zipWithIndex.toMap
Ordering.by(e => orderedPorts(e))
}

def apply(s: String): Option[MemPort] = MemPort.all.find(_.name == s)

Expand All @@ -38,7 +52,8 @@ case class MemConf(
ports: Map[MemPort, Int],
maskGranularity: Option[Int]) {

private def portsStr = ports.map { case (port, num) => Seq.fill(num)(port.name).mkString(",") }.mkString(",")
private def portsStr =
ports.toSeq.sortBy(_._1).map { case (port, num) => Seq.fill(num)(port.name).mkString(",") }.mkString(",")
private def maskGranStr = maskGranularity.map((p) => s"mask_gran $p").getOrElse("")

// Assert that all of the entries in the port map are greater than zero to make it easier to compare two of these case classes
Expand Down
12 changes: 12 additions & 0 deletions src/test/scala/firrtlTests/ReplSeqMemTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -693,4 +693,16 @@ circuit Top :
|""".stripMargin
compileAndEmit(CircuitState(parse(input), ChirrtlForm))
}

"MemPorts" should "serialize in a deterministic order regardless" in {
def compare(seq1: Seq[MemPort]) {
val m1 = MemConf("memconf", 8, 16, seq1.map(s => s -> 1).toMap, None)
val m2 = MemConf("memconf", 8, 16, seq1.reverse.map(s => s -> 1).toMap, None)
m1.toString should be(m2.toString)
}

compare(Seq(ReadPort, WritePort))
compare(Seq(MaskedWritePort, ReadWritePort))
compare(Seq(MaskedReadWritePort, WritePort, ReadWritePort))
}
}

0 comments on commit f7b4aa8

Please sign in to comment.