From f7b4aa8a3a759c94e0de8e1aeeacc69d3d702945 Mon Sep 17 00:00:00 2001
From: Chick Markley <chick.markley@sifive.com>
Date: Thu, 7 Apr 2022 17:07:48 -0700
Subject: [PATCH] Make MemConf's MemPort serialization deterministic (#2508)

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.
---
 .../scala/firrtl/passes/memlib/MemConf.scala  | 19 +++++++++++++++++--
 .../scala/firrtlTests/ReplSeqMemTests.scala   | 12 ++++++++++++
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/src/main/scala/firrtl/passes/memlib/MemConf.scala b/src/main/scala/firrtl/passes/memlib/MemConf.scala
index 637b57e0f6..8063c62787 100644
--- a/src/main/scala/firrtl/passes/memlib/MemConf.scala
+++ b/src/main/scala/firrtl/passes/memlib/MemConf.scala
@@ -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)
 
@@ -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
diff --git a/src/test/scala/firrtlTests/ReplSeqMemTests.scala b/src/test/scala/firrtlTests/ReplSeqMemTests.scala
index d9dc2e5779..e8d72043de 100644
--- a/src/test/scala/firrtlTests/ReplSeqMemTests.scala
+++ b/src/test/scala/firrtlTests/ReplSeqMemTests.scala
@@ -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))
+  }
 }