Improve read-under-write parameter support

* Make the read-under-write (RUW) parameter typesafe
* Add RUW support to the FIRRTL proto and CHIRRTL grammar

10 files changed, 72 insertions, 16 deletions

diff --git a/src/main/antlr4/FIRRTL.g4 b/src/main/antlr4/FIRRTL.g4
index c3b4e74e..be15ab7c 100644
--- a/src/main/antlr4/FIRRTL.g4
+++ b/src/main/antlr4/FIRRTL.g4
@@ -94,7 +94,7 @@ stmt
   | 'reg' id ':' type exp ('with' ':' reset_block)? info?
   | 'mem' id ':' info? INDENT memField* DEDENT
   | 'cmem' id ':' type info?
-  | 'smem' id ':' type info?
+  | 'smem' id ':' type ruw? info?
   | mdir 'mport' id '=' id '[' exp ']' exp info?
   | 'inst' id 'of' id info?
   | 'node' id '=' exp info?
diff --git a/src/main/proto/firrtl.proto b/src/main/proto/firrtl.proto
index b8f6db98..0cf14f41 100644
--- a/src/main/proto/firrtl.proto
+++ b/src/main/proto/firrtl.proto
@@ -104,6 +104,12 @@ message Firrtl {
       Expression init = 5;
     }
 
+    enum ReadUnderWrite {
+      UNDEFINED = 0;
+      OLD = 1;
+      NEW = 2;
+    }
+
     message Memory {
       // Required.
       string id = 1;
@@ -121,6 +127,7 @@ message Firrtl {
       repeated string reader_id = 6;
       repeated string writer_id = 7;
       repeated string readwriter_id = 8;
+      ReadUnderWrite read_under_write = 10;
     }
 
     message CMemory {
@@ -138,6 +145,7 @@ message Firrtl {
       }
       // Required.
       bool sync_read = 3;
+      ReadUnderWrite read_under_write = 5;
     }
 
     message Instance {
diff --git a/src/main/scala/firrtl/Visitor.scala b/src/main/scala/firrtl/Visitor.scala
index 6d9f0d31..01de8f15 100644
--- a/src/main/scala/firrtl/Visitor.scala
+++ b/src/main/scala/firrtl/Visitor.scala
@@ -164,15 +164,23 @@ class Visitor(infoMode: InfoMode) extends AbstractParseTreeVisitor[FirrtlNode] w
   private def visitSuite(ctx: SuiteContext): Statement =
     Block(ctx.simple_stmt().asScala.flatMap(x => Option(x.stmt).map(visitStmt)))
 
+  private def visitRuw(ctx: Option[RuwContext]): ReadUnderWrite.Value = ctx match {
+    case None => ReadUnderWrite.Undefined
+    case Some(ctx) => ctx.getText match {
+      case "undefined" => ReadUnderWrite.Undefined
+      case "old" => ReadUnderWrite.Old
+      case "new" => ReadUnderWrite.New
+    }
+  }
 
   // Memories are fairly complicated to translate thus have a dedicated method
   private def visitMem(ctx: StmtContext): Statement = {
     val readers = mutable.ArrayBuffer.empty[String]
     val writers = mutable.ArrayBuffer.empty[String]
     val readwriters = mutable.ArrayBuffer.empty[String]
-    case class ParamValue(typ: Option[Type] = None, lit: Option[BigInt] = None, ruw: Option[String] = None, unique: Boolean = true)
+    case class ParamValue(typ: Option[Type] = None, lit: Option[BigInt] = None, ruw: ReadUnderWrite.Value = ReadUnderWrite.Undefined, unique: Boolean = true)
     val fieldMap = mutable.HashMap[String, ParamValue]()
-
+    val memName = ctx.id(0).getText
     def parseMemFields(memFields: Seq[MemFieldContext]): Unit =
       memFields.foreach { field =>
         val fieldName = field.children.asScala(0).getText
@@ -184,7 +192,7 @@ class Visitor(infoMode: InfoMode) extends AbstractParseTreeVisitor[FirrtlNode] w
           case _ =>
             val paramDef = fieldName match {
               case "data-type" => ParamValue(typ = Some(visitType(field.`type`())))
-              case "read-under-write" => ParamValue(ruw = Some(field.ruw().getText)) // TODO
+              case "read-under-write" => ParamValue(ruw = visitRuw(Option(field.ruw)))
               case _ => ParamValue(lit = Some(BigInt(field.intLit().getText)))
             }
             if (fieldMap.contains(fieldName))
@@ -210,10 +218,11 @@ class Visitor(infoMode: InfoMode) extends AbstractParseTreeVisitor[FirrtlNode] w
     }
 
     def lit(param: String) = fieldMap(param).lit.get
-    val ruw = fieldMap.get("read-under-write").map(_.ruw).getOrElse(None)
+    val ruw = fieldMap.get("read-under-write").map(_.ruw).getOrElse(ir.ReadUnderWrite.Undefined)
 
     DefMemory(info,
-      name = ctx.id(0).getText, dataType = fieldMap("data-type").typ.get,
+      name = memName,
+      dataType = fieldMap("data-type").typ.get,
       depth = lit("depth"),
       writeLatency = lit("write-latency").toInt,
       readLatency = lit("read-latency").toInt,
@@ -269,7 +278,7 @@ class Visitor(infoMode: InfoMode) extends AbstractParseTreeVisitor[FirrtlNode] w
           CDefMemory(info, ctx.id(0).getText, tpe, size, seq = false)
         case "smem" =>
           val (tpe, size) = visitCMemType(ctx.`type`())
-          CDefMemory(info, ctx.id(0).getText, tpe, size, seq = true)
+          CDefMemory(info, ctx.id(0).getText, tpe, size, seq = true, readUnderWrite = visitRuw(Option(ctx.ruw)))
         case "inst" => DefInstance(info, ctx.id(0).getText, ctx.id(1).getText)
         case "node" => DefNode(info, ctx.id(0).getText, visitExp(ctx_exp(0)))
 
diff --git a/src/main/scala/firrtl/WIR.scala b/src/main/scala/firrtl/WIR.scala
index 73a20d18..475f5e9c 100644
--- a/src/main/scala/firrtl/WIR.scala
+++ b/src/main/scala/firrtl/WIR.scala
@@ -341,7 +341,8 @@ case class CDefMemory(
     name: String,
     tpe: Type,
     size: BigInt,
-    seq: Boolean) extends Statement with HasInfo {
+    seq: Boolean,
+    readUnderWrite: ReadUnderWrite.Value = ReadUnderWrite.Undefined) extends Statement with HasInfo {
   def serialize: String = (if (seq) "smem" else "cmem") +
     s" $name : ${tpe.serialize} [$size]" + info.serialize
   def mapExpr(f: Expression => Expression): Statement = this
diff --git a/src/main/scala/firrtl/ir/IR.scala b/src/main/scala/firrtl/ir/IR.scala
index 9268865b..b4631473 100644
--- a/src/main/scala/firrtl/ir/IR.scala
+++ b/src/main/scala/firrtl/ir/IR.scala
@@ -285,6 +285,13 @@ case class DefInstance(info: Info, name: String, module: String) extends Stateme
   def foreachString(f: String => Unit): Unit = f(name)
   def foreachInfo(f: Info => Unit): Unit = f(info)
 }
+
+object ReadUnderWrite extends Enumeration {
+  val Undefined = Value("undefined")
+  val Old = Value("old")
+  val New = Value("new")
+}
+
 case class DefMemory(
     info: Info,
     name: String,
@@ -296,7 +303,7 @@ case class DefMemory(
     writers: Seq[String],
     readwriters: Seq[String],
     // TODO: handle read-under-write
-    readUnderWrite: Option[String] = None) extends Statement with IsDeclaration {
+    readUnderWrite: ReadUnderWrite.Value = ReadUnderWrite.Undefined) extends Statement with IsDeclaration {
   def serialize: String =
     s"mem $name :" + info.serialize +
     indent(
@@ -307,7 +314,7 @@ case class DefMemory(
           (readers map ("reader => " + _)) ++
           (writers map ("writer => " + _)) ++
           (readwriters map ("readwriter => " + _)) ++
-       Seq("read-under-write => undefined")) mkString "\n")
+       Seq(s"read-under-write => ${readUnderWrite}")) mkString "\n")
   def mapStmt(f: Statement => Statement): Statement = this
   def mapExpr(f: Expression => Expression): Statement = this
   def mapType(f: Type => Type): Statement = this.copy(dataType = f(dataType))
diff --git a/src/main/scala/firrtl/passes/RemoveCHIRRTL.scala b/src/main/scala/firrtl/passes/RemoveCHIRRTL.scala
index d0498cf0..5e93b3b9 100644
--- a/src/main/scala/firrtl/passes/RemoveCHIRRTL.scala
+++ b/src/main/scala/firrtl/passes/RemoveCHIRRTL.scala
@@ -99,7 +99,7 @@ object RemoveCHIRRTL extends Transform {
         set_enable(rws, "en") ++
         set_write(rws, "wdata", "wmask")
       val mem = DefMemory(sx.info, sx.name, sx.tpe, sx.size, 1, if (sx.seq) 1 else 0,
-                  rds map (_.name), wrs map (_.name), rws map (_.name))
+                  rds map (_.name), wrs map (_.name), rws map (_.name), sx.readUnderWrite)
       Block(mem +: stmts)
     case sx: CDefMPort =>
       types.get(sx.mem) match {
diff --git a/src/main/scala/firrtl/passes/memlib/MemIR.scala b/src/main/scala/firrtl/passes/memlib/MemIR.scala
index 2379feab..afba7535 100644
--- a/src/main/scala/firrtl/passes/memlib/MemIR.scala
+++ b/src/main/scala/firrtl/passes/memlib/MemIR.scala
@@ -34,7 +34,7 @@ case class DefAnnotatedMemory(
     readers: Seq[String],
     writers: Seq[String],
     readwriters: Seq[String],
-    readUnderWrite: Option[String],
+    readUnderWrite: ReadUnderWrite.Value,
     maskGran: Option[BigInt],
     memRef: Option[(String, String)] /* (Module, Mem) */
     //pins: Seq[Pin],
diff --git a/src/main/scala/firrtl/proto/FromProto.scala b/src/main/scala/firrtl/proto/FromProto.scala
index 22c90316..ef2ee5bd 100644
--- a/src/main/scala/firrtl/proto/FromProto.scala
+++ b/src/main/scala/firrtl/proto/FromProto.scala
@@ -8,6 +8,7 @@ import java.io.{File, FileInputStream, InputStream}
 import collection.JavaConverters._
 import FirrtlProtos._
 import com.google.protobuf.CodedInputStream
+import Firrtl.Statement.ReadUnderWrite
 
 object FromProto {
 
@@ -133,6 +134,12 @@ object FromProto {
     ir.Conditionally(convert(info), convert(when.getPredicate), conseq, alt)
   }
 
+  def convert(ruw: ReadUnderWrite): ir.ReadUnderWrite.Value = ruw match {
+    case ReadUnderWrite.UNDEFINED => ir.ReadUnderWrite.Undefined
+    case ReadUnderWrite.OLD => ir.ReadUnderWrite.Old
+    case ReadUnderWrite.NEW => ir.ReadUnderWrite.New
+  }
+
   def convert(dt: Firrtl.Statement.CMemory.TypeAndDepth): (ir.Type, BigInt) =
     (convert(dt.getDataType), convert(dt.getDepth))
 
@@ -145,7 +152,7 @@ object FromProto {
       case TYPE_AND_DEPTH_FIELD_NUMBER =>
         convert(cmem.getTypeAndDepth)
     }
-    CDefMemory(convert(info), cmem.getId, tpe, depth, cmem.getSyncRead)
+    CDefMemory(convert(info), cmem.getId, tpe, depth, cmem.getSyncRead, convert(cmem.getReadUnderWrite))
   }
 
   import Firrtl.Statement.MemoryPort.Direction._
@@ -181,7 +188,7 @@ object FromProto {
       case BIGINT_DEPTH_FIELD_NUMBER => convert(mem.getBigintDepth)
     }
     ir.DefMemory(convert(info), mem.getId, dtype, depth, mem.getWriteLatency, mem.getReadLatency,
-                 rs, ws, rws, None)
+                 rs, ws, rws, convert(mem.getReadUnderWrite))
   }
 
   def convert(attach: Firrtl.Statement.Attach, info: Firrtl.SourceInfo): ir.Attach = {
diff --git a/src/main/scala/firrtl/proto/ToProto.scala b/src/main/scala/firrtl/proto/ToProto.scala
index 17adb698..70de3ccd 100644
--- a/src/main/scala/firrtl/proto/ToProto.scala
+++ b/src/main/scala/firrtl/proto/ToProto.scala
@@ -6,6 +6,7 @@ package proto
 import java.io.OutputStream
 
 import FirrtlProtos._
+import Firrtl.Statement.ReadUnderWrite
 import Firrtl.Expression.PrimOp.Op
 import com.google.protobuf.{CodedOutputStream, WireFormat}
 import firrtl.PrimOps._
@@ -103,6 +104,12 @@ object ToProto {
     BPSet -> Op.OP_SET_BINARY_POINT
   )
 
+  def convert(ruw: ir.ReadUnderWrite.Value): ReadUnderWrite = ruw match {
+    case ir.ReadUnderWrite.Undefined => ReadUnderWrite.UNDEFINED
+    case ir.ReadUnderWrite.Old => ReadUnderWrite.OLD
+    case ir.ReadUnderWrite.New => ReadUnderWrite.NEW
+  }
+
   def convertToIntegerLiteral(value: BigInt): Firrtl.Expression.IntegerLiteral.Builder = {
     Firrtl.Expression.IntegerLiteral.newBuilder()
       .setValue(value.toString)
@@ -260,22 +267,24 @@ object ToProto {
             val ib = Firrtl.Statement.IsInvalid.newBuilder()
               .setExpression(convert(expr))
             sb.setIsInvalid(ib)
-          case ir.DefMemory(_, name, dtype, depth, wlat, rlat, rs, ws, rws, _) =>
+          case ir.DefMemory(_, name, dtype, depth, wlat, rlat, rs, ws, rws, ruw) =>
             val mem = Firrtl.Statement.Memory.newBuilder()
               .setId(name)
               .setType(convert(dtype))
               .setBigintDepth(convertToBigInt(depth))
               .setWriteLatency(wlat)
               .setReadLatency(rlat)
+              .setReadUnderWrite(convert(ruw))
             mem.addAllReaderId(rs.asJava)
             mem.addAllWriterId(ws.asJava)
             mem.addAllReadwriterId(rws.asJava)
             sb.setMemory(mem)
-          case CDefMemory(_, name, tpe, size, seq) =>
+          case CDefMemory(_, name, tpe, size, seq, ruw) =>
             val mb = Firrtl.Statement.CMemory.newBuilder()
               .setId(name)
               .setTypeAndDepth(convert(tpe, size))
               .setSyncRead(seq)
+              .setReadUnderWrite(convert(ruw))
             sb.setCmemory(mb)
           case CDefMPort(_, name, _, mem, exprs, dir) =>
             val pb = Firrtl.Statement.MemoryPort.newBuilder()
diff --git a/src/test/scala/firrtlTests/ProtoBufSpec.scala b/src/test/scala/firrtlTests/ProtoBufSpec.scala
index 2f347c6d..7f41fb26 100644
--- a/src/test/scala/firrtlTests/ProtoBufSpec.scala
+++ b/src/test/scala/firrtlTests/ProtoBufSpec.scala
@@ -176,6 +176,21 @@ class ProtoBufSpec extends FirrtlFlatSpec {
     oldCMem should equal (cmem)
   }
 
+  // readunderwrite support
+  it should "support readunderwrite parameters" in {
+    val m1 = DefMemory(NoInfo, "m", UIntType(IntWidth(8)), 128, 1, 1, List("r"), List("w"), Nil, ir.ReadUnderWrite.Old)
+    FromProto.convert(ToProto.convert(m1).head.build) should equal (m1)
+
+    val m2 = m1.copy(readUnderWrite = ir.ReadUnderWrite.New)
+    FromProto.convert(ToProto.convert(m2).head.build) should equal (m2)
+
+    val cm1 = CDefMemory(NoInfo, "m", UIntType(IntWidth(8)), 128, true, ir.ReadUnderWrite.Old)
+    FromProto.convert(ToProto.convert(cm1).head.build) should equal (cm1)
+
+    val cm2 = cm1.copy(readUnderWrite = ir.ReadUnderWrite.New)
+    FromProto.convert(ToProto.convert(cm2).head.build) should equal (cm2)
+  }
+
   it should "support AsyncResetTypes" in {
     val port = ir.Port(ir.NoInfo, "reset", ir.Input, ir.AsyncResetType)
     FromProto.convert(ToProto.convert(port).build) should equal (port)