Sint tests and change in serialization (#456)

SInt representation is no longer 2's complement, but instead a positive number (hex or base 10) that is optionally preceded by a sign (-+).

9 files changed, 118 insertions, 78 deletions

diff --git a/spec/spec.pdf b/spec/spec.pdf
index bf5e4d23..73bba098 100644
--- a/spec/spec.pdf
+++ b/spec/spec.pdf
diff --git a/spec/spec.tex b/spec/spec.tex
index 7e9188e2..54c4c4de 100644
--- a/spec/spec.tex
+++ b/spec/spec.tex
@@ -1,4 +1,4 @@
-% See LICENSE for license details.
+
 %Useful Macros
 \newcommand{\id}{\text{id }}
 \newcommand{\idst}{\text{id*}}
@@ -911,24 +911,24 @@ A literal unsigned integer can alternatively be created given a string represent
 
 The following radices are supported:
 \begin{enumerate}
-\item \verb|0b| : For representing binary numbers.
-\item \verb|0o| : For representing octal numbers.
-\item \verb|0x| : For representing hexadecimal numbers.
+\item \verb|b| : For representing binary numbers.
+\item \verb|o| : For representing octal numbers.
+\item \verb|h| : For representing hexadecimal numbers.
 \end{enumerate}
 
 If a bit width is not given, the number of bits in the bit representation is directly represented by the string. The following examples create a 8-bit integer representing the number 13.
 \begin{verbatim}
-UBits("0b00001101")
-UBits("0x0D")
+UInt("b00001101")
+UInt("h0D")
 \end{verbatim}
 
-If the provided bit width is larger than the number of bits represented by the string, then the resulting value is equivalent to the string zero-extended up to the provided bit width. If the provided bit width is smaller than the number of bits represented by the string, then the resulting value is equivalent to the string truncated down to the provided bit width. All truncated bits must be zero.
+If the provided bit width is larger than the number of bits required to represent the string's value, then the resulting value is equivalent to the string zero-extended up to the provided bit width. If the provided bit width is smaller than the number of bits represented by the string, then the resulting value is equivalent to the string truncated down to the provided bit width. All truncated bits must be zero.
 
 The following examples create a 7-bit integer representing the number 13.
 \begin{verbatim}
-UBits<7>("0b00001101")
-UBits<7>("0o015")
-UBits<7>("0xD")
+UInt<7>("b00001101")
+UInt<7>("o015")
+UInt<7>("hD")
 \end{verbatim}
 
 \subsection{Signed Integers}
@@ -945,23 +945,19 @@ SInt(-42)
 
 \subsection{Signed Integers from Literal Bits}
 
-Similar to unsigned integers, a literal signed integer can alternatively be created given a string representing its bit representation and an optional bit width. 
+Similar to unsigned integers, a literal signed integer can alternatively be created given a string representing its bit representation and an optional bit width.
 
-If a bit width is not given, the number of bits in the bit representation is directly represented by the string. The following examples create a 8-bit integer representing the number -13.
-\begin{verbatim}
-SBits("0b11110011")
-SBits("0xF3")
-\end{verbatim}
+The bit representation contains a binary, octal or hex indicator, followed by an optional sign, followed by the value.
 
-If the provided bit width is larger than the number of bits represented by the string, then the resulting value is equivalent to the string sign-extended up to the provided bit width. If the provided bit width is smaller than the number of bits represented by the string, then the resulting value is equivalent to the string truncated down to the provided bit width. All truncated bits must match the sign bit of the final truncated number.
-
-The following examples create a 7-bit integer representing the number -13.
+If a bit width is not given, the number of bits in the bit representation is the minimal bitwidth to represent the value represented by the string. The following examples create a 8-bit integer representing the number -13.
 \begin{verbatim}
-SBits<7>("0b10011")
-SBits<7>("0o763")
-SBits<7>("0xF3")
+SInt("b-1101")
+SInt("h-d")
 \end{verbatim}
 
+If the provided bit width is larger than the number of bits represented by the string, then the resulting value is unchanged.
+It is an error to provide a bit width smaller than the number of bits required to represent the string's value.
+
 \subsection{References}
 A reference is simply a name that refers to a previously declared circuit component. It may refer to a module port, node, wire, register, instance, or memory.
 
@@ -1719,20 +1715,22 @@ The concrete syntax of FIRRTL is defined in section \ref{syntax_tree}. Productio
 \subsection{Concrete Syntax Tree} \label{syntax_tree}
 
 \begin{tabular}{rrll}
-\pd{circuit}    &=     &\opt{\pd{info}} \vv{circuit} \pd{id} \vv{:} \vv{(}\rpt{\pd{module}}\vv{)}              &\text{Circuit}\\
-\pd{module}     &=     &\opt{\pd{info}} \vv{module} \pd{id} \vv{:} \vv{(}\rpt{\pd{port}} \pd{stmt}\vv{)}       &\text{Module}\\
-                &\pipe &\opt{\pd{info}} \vv{extmodule} \pd{id} \vv{:} \vv{(}\rpt{\pd{port}}\vv{)}              &\text{External Module}\\
-\pd{port}       &=     &\opt{\pd{info}} \pd{dir} \pd{id} \vv{:} \pd{type}                                      &\text{Port}\\
+\pd{circuit}    &=     &\vv{circuit} \pd{id} \vv{:} \opt{\pd{info}} \vv{(}\rpt{\pd{module}}\vv{)}              &\text{Circuit}\\
+\pd{module}     &=     &\vv{module} \pd{id} \vv{:} \opt{\pd{info}} \vv{(}\rpt{\pd{port}} \pd{stmt}\vv{)}       &\text{Module}\\
+                &\pipe &\vv{extmodule} \pd{id} \vv{:} \opt{\pd{info}} \vv{(}\rpt{\pd{port}}\vv{)}              &\text{External Module}\\
+\pd{port}       &=     &\pd{dir} \pd{id} \vv{:} \pd{type} \opt{\pd{info}}                                      &\text{Port}\\
 \pd{dir}        &=     &\vv{input} \pipe \vv{ output}                                                          &\text{Port Direction}\\
 \pd{type}       &=     &\vv{UInt}\opt{\vv{<}\pd{int}\vv{>}}                                                    &\text{Unsigned Integer}\\
                 &\pipe &\vv{SInt}\opt{\vv{<}\pd{int}\vv{>}}                                                    &\text{Signed Integer}\\
+                &\pipe &\vv{Fixed}\opt{\vv{<}\pd{int}\vv{>}}\opt{\vv{<<}\pd{int}\vv{>>}}                       &\text{Fixed Point}\\
                 &\pipe &\vv{Clock}                                                                             &\text{Clock}\\
+                &\pipe &\vv{Analog}\opt{\vv{<}\pd{int}\vv{>}}                                                  &\text{Analog}\\
                 &\pipe &\verb|{|\rpt{\pd{field}}\verb|}|                                                       &\text{Bundle}\\
                 &\pipe &\pd{type}\vv{[}\pd{int}\vv{]}                                                          &\text{Vector}\\
 \pd{field}      &=     &\opt{\vv{flip}} \pd{id} \vv{:} \pd{type}                                               &\text{Bundle Field}\\
-\pd{stmt}       &=     &\opt{\pd{info}} \vv{wire} \pd{id} \vv{:} \pd{type}                                     &\text{Wire}\\
-                &\pipe &\opt{\pd{info}} \vv{reg} \pd{id} \vv{:} \pd{type} \pd{exp} \opt{\vv{(with:} \vv{\{reset => (}\pd{exp}, \pd{exp}\vv{)\})}}   &\text{Register}\\
-                &\pipe &\opt{\pd{info}} \vv{mem} \pd{id} \vv{:} \vv{(}                                         &\text{Memory}\\
+\pd{stmt}       &=     &\vv{wire} \pd{id} \vv{:} \pd{type} \opt{\pd{info}}                                     &\text{Wire}\\
+                &\pipe &\vv{reg} \pd{id} \vv{:} \pd{type} \pd{exp} \opt{\vv{(with:} \vv{\{reset => (}\pd{exp}, \pd{exp}\vv{)\})}} \opt{\pd{info}}   &\text{Register}\\
+                &\pipe &\vv{mem} \pd{id} \vv{:} \opt{\pd{info}} \vv{(}                                         &\text{Memory}\\
                 && \verb|       |\vv{data-type =>} \pd{type}\\
                 && \verb|       |\vv{depth =>} \pd{int}\\                
                 && \verb|       |\vv{read-latency =>} \pd{int}\\                
@@ -1741,16 +1739,17 @@ The concrete syntax of FIRRTL is defined in section \ref{syntax_tree}. Productio
                 && \verb|       |\rpt{\vv{reader =>} \pd{id}}\\             
                 && \verb|       |\rpt{\vv{writer =>} \pd{id}}\\    
                 && \verb|       |\rpt{\vv{readwriter =>} \pd{id}}\vv{)}                                \\
-                &\pipe &\opt{\pd{info}} \vv{inst} \pd{id} \vv{of} \pd{id}                                      &\text{Instance}\\
-                &\pipe &\opt{\pd{info}} \vv{node} \pd{id} \vv{=} \pd{exp}                                      &\text{Node}\\
-                &\pipe &\opt{\pd{info}} \pd{exp} \vv{<=} \pd{exp}                                              &\text{Connect}\\
-                &\pipe &\opt{\pd{info}} \pd{exp} \vv{<-} \pd{exp}                                              &\text{Partial Connect}\\
-                &\pipe &\opt{\pd{info}} \pd{exp} \vv{is invalid}                                               &\text{Invalidate}\\
-                &\pipe &\opt{\pd{info}} \vv{when} \pd{exp} \vv{:} \pd{stmt} \opt{\vv{else} \vv{:} \pd{stmt}}   &\text{Conditional}\\
-                &\pipe &\opt{\pd{info}} \vv{stop(}\pd{exp}, \pd{exp}, \pd{int})                                &\text{Stop}\\
-                &\pipe &\opt{\pd{info}} \vv{printf(}\pd{exp}, \pd{exp}, \pd{string}, \rpt{\pd{exp}}\vv{)}      &\text{Printf}\\
-                &\pipe &\opt{\pd{info}} \vv{skip}                                                              &\text{Empty}\\
-                &\pipe &\opt{\pd{info}} \vv{(}\rpt{\pd{stmt}}\vv{)}                                            &\text{Statement Group}\\
+                &\pipe &\vv{inst} \pd{id} \vv{of} \pd{id} \opt{\pd{info}}                                      &\text{Instance}\\
+                &\pipe &\vv{node} \pd{id} \vv{=} \pd{exp} \opt{\pd{info}}                                      &\text{Node}\\
+                &\pipe &\pd{exp} \vv{<=} \pd{exp} \opt{\pd{info}}                                              &\text{Connect}\\
+                &\pipe &\pd{exp} \vv{<-} \pd{exp} \opt{\pd{info}}                                              &\text{Partial Connect}\\
+                &\pipe &\pd{exp} \vv{is invalid} \opt{\pd{info}}                                               &\text{Invalidate}\\
+                &\pipe &\vv{attach}\vv{(}\rpt{\pd{exp}}\vv{)} \opt{\pd{info}}                                  &\text{Attach}\\
+                &\pipe &\vv{when} \pd{exp} \vv{:} \opt{\pd{info}} \pd{stmt} \opt{\vv{else} \vv{:} \pd{stmt}}   &\text{Conditional}\\
+                &\pipe &\vv{stop(}\pd{exp}, \pd{exp}, \pd{int})\opt{\pd{info}}                                 &\text{Stop}\\
+                &\pipe &\vv{printf(}\pd{exp}, \pd{exp}, \pd{string}, \rpt{\pd{exp}}\vv{)} \opt{\pd{info}}      &\text{Printf}\\
+                &\pipe &\vv{skip} \opt{\pd{info}}                                                              &\text{Empty}\\
+                &\pipe &\vv{(}\rpt{\pd{stmt}}\vv{)}                                                            &\text{Statement Group}\\
 \pd{ruw}        &=     &\vv{old} \pipe \vv{ new} \pipe \vv{ undefined}                                         &\text{Read Under Write Flag}\\
 \pd{info}       &=     &\vv{@[}\pd{string}\vv{]}                                                               &\text{File Information Token}\\
 \end{tabular}
@@ -1807,18 +1806,9 @@ The concrete syntax of FIRRTL is defined in section \ref{syntax_tree}. Productio
 %\section{TODO}
 %
 %- FIRRTL implementation
-%   - Add UBits                         ; andrew doesn't care, favors overloading UInt
-%   - Add SBits
 %   - Add partial connect algorithm     ; 
 %   - Add oriented types to type checker
 %   - Add memory read-under-write flag  ; probably overengineering, but could be a wash
-%   - *FINISHED* Make register reset/init optional ; good
-%   - *FINISHED* removed addw, added head and tail ; great!
-%   - *FINISHED* Rework readwrite port types       ; limits optimizations but probably ok
-%   - *FINISHED* Add Mux expression                ; that's lovely, need glitch-free mux for clock types
-%   - *FINISHED* add rename pass for verilog
-%   - *FINISHED* Add is invalid                    ; good
-%   - *FINISHED* Add validif                       ; good
 
 \end{document}
 
diff --git a/src/main/antlr4/FIRRTL.g4 b/src/main/antlr4/FIRRTL.g4
index cdec7260..e26aca2e 100644
--- a/src/main/antlr4/FIRRTL.g4
+++ b/src/main/antlr4/FIRRTL.g4
@@ -297,7 +297,7 @@ PosInt
   ;
 
 HexLit
-  : '"' 'h' ( HexDigit )+ '"'
+  : '"' 'h' ( '+' | '-' )? ( HexDigit )+ '"'
   ;
 
 DoubleLit
diff --git a/src/main/scala/firrtl/Emitter.scala b/src/main/scala/firrtl/Emitter.scala
index b1c318fa..1153b1e6 100644
--- a/src/main/scala/firrtl/Emitter.scala
+++ b/src/main/scala/firrtl/Emitter.scala
@@ -248,8 +248,11 @@ class VerilogEmitter extends Transform with PassBased with Emitter {
      case UIntLiteral(value, IntWidth(width)) =>
        w write s"$width'h${value.toString(16)}"
      case SIntLiteral(value, IntWidth(width)) =>
-       val unsignedValue = value + (if (value < 0) BigInt(1) << width.toInt else 0)
-       w write s"$width'sh${unsignedValue.toString(16)}"
+       val stringLiteral = value.toString(16)
+       w write (stringLiteral.head match {
+         case '-' => s"-$width'sh${stringLiteral.tail}"
+         case _ => s"$width'sh${stringLiteral}"
+       })
    }
 
    def op_stream(doprim: DoPrim): Seq[Any] = {
diff --git a/src/main/scala/firrtl/Utils.scala b/src/main/scala/firrtl/Utils.scala
index c13e031e..ec53aa63 100644
--- a/src/main/scala/firrtl/Utils.scala
+++ b/src/main/scala/firrtl/Utils.scala
@@ -154,9 +154,6 @@ object Utils extends LazyLogging {
 
   /** Indent the results of [[ir.FirrtlNode.serialize]] */
   def indent(str: String) = str replaceAllLiterally ("\n", "\n  ")
-  def serialize(bi: BigInt): String =
-    if (bi < BigInt(0)) "\"h" + bi.toString(16).substring(1) + "\""
-    else "\"h" + bi.toString(16) + "\""
 
   implicit def toWrappedExpression (x:Expression): WrappedExpression = new WrappedExpression(x)
   def ceilLog2(x: BigInt): Int = (x-1).bitLength
diff --git a/src/main/scala/firrtl/Visitor.scala b/src/main/scala/firrtl/Visitor.scala
index 73555a37..222c2792 100644
--- a/src/main/scala/firrtl/Visitor.scala
+++ b/src/main/scala/firrtl/Visitor.scala
@@ -26,24 +26,16 @@ class Visitor(infoMode: InfoMode) extends FIRRTLBaseVisitor[FirrtlNode] {
   def visit[FirrtlNode](ctx: FIRRTLParser.CircuitContext): Circuit = visitCircuit(ctx)
 
   //  These regex have to change if grammar changes
+  private val HexPattern = """\"*h([+\-]?[a-zA-Z0-9]+)\"*""".r
+  private val DecPattern = """([+\-]?[1-9]\d*)""".r
+  private val ZeroPattern = "0".r
+
   private def string2BigInt(s: String): BigInt = {
     // private define legal patterns
-    val HexPattern =
-      """\"*h([a-zA-Z0-9]+)\"*""".r
-    val DecPattern = """(\+|-)?([1-9]\d*)""".r
-    val ZeroPattern = "0".r
-    val NegPattern = "(89AaBbCcDdEeFf)".r
     s match {
       case ZeroPattern(_*) => BigInt(0)
-      case HexPattern(hexdigits) =>
-        hexdigits(0) match {
-          case NegPattern(_) =>
-            BigInt("-" + hexdigits, 16)
-          case _ => BigInt(hexdigits, 16)
-        }
-      case DecPattern(sign, num) =>
-        if (sign != null) BigInt(sign + num, 10)
-        else BigInt(num, 10)
+      case HexPattern(hexdigits) => BigInt(hexdigits, 16)
+      case DecPattern(num) => BigInt(num, 10)
       case _ => throw new Exception("Invalid String for conversion to BigInt " + s)
     }
   }
@@ -310,11 +302,19 @@ class Visitor(infoMode: InfoMode) extends FIRRTLBaseVisitor[FirrtlNode] {
           UIntLiteral(value, width)
         case "SInt" =>
           val (width, value) =
-            if (ctx.getChildCount > 4)
-              (IntWidth(string2BigInt(ctx.intLit(0).getText)), string2BigInt(ctx.intLit(1).getText))
-            else {
-              val bigint = string2BigInt(ctx.intLit(0).getText)
-              (IntWidth(BigInt(bigint.bitLength + 1)), bigint)
+            if (ctx.getChildCount > 4) {
+              val width = string2BigInt(ctx.intLit(0).getText)
+              val value = string2BigInt(ctx.intLit(1).getText)
+              (IntWidth(width), value)
+            } else {
+              val str = ctx.intLit(0).getText
+              val value = string2BigInt(str)
+              // To calculate bitwidth of negative number,
+              //  1) negate number and subtract one to get the maximum positive value.
+              //  2) get bitwidth of max positive number
+              //  3) add one to account for the signed representation
+              val width = if (value < 0) (value.abs - BigInt(1)).bitLength + 1 else value.bitLength + 1
+              (IntWidth(BigInt(width)), value)
             }
           SIntLiteral(value, width)
         case "validif(" => ValidIf(visitExp(ctx.exp(0)), visitExp(ctx.exp(1)), UnknownType)
diff --git a/src/main/scala/firrtl/ir/IR.scala b/src/main/scala/firrtl/ir/IR.scala
index 71ad9366..938b5848 100644
--- a/src/main/scala/firrtl/ir/IR.scala
+++ b/src/main/scala/firrtl/ir/IR.scala
@@ -103,14 +103,14 @@ abstract class Literal extends Expression {
 }
 case class UIntLiteral(value: BigInt, width: Width) extends Literal {
   def tpe = UIntType(width)
-  def serialize = s"UInt${width.serialize}(" + Utils.serialize(value) + ")"
+  def serialize = s"""UInt${width.serialize}("h""" + value.toString(16)+ """")"""
   def mapExpr(f: Expression => Expression): Expression = this
   def mapType(f: Type => Type): Expression = this
   def mapWidth(f: Width => Width): Expression = UIntLiteral(value, f(width))
 }
 case class SIntLiteral(value: BigInt, width: Width) extends Literal {
   def tpe = SIntType(width)
-  def serialize = s"SInt${width.serialize}(" + Utils.serialize(value) + ")"
+  def serialize = s"""SInt${width.serialize}("h""" + value.toString(16)+ """")"""
   def mapExpr(f: Expression => Expression): Expression = this
   def mapType(f: Type => Type): Expression = this
   def mapWidth(f: Width => Width): Expression = SIntLiteral(value, f(width))
@@ -119,7 +119,7 @@ case class FixedLiteral(value: BigInt, width: Width, point: Width) extends Liter
   def tpe = FixedType(width, point)
   def serialize = {
     val pstring = if(point == UnknownWidth) "" else s"<${point.serialize}>"
-    s"Fixed${width.serialize}$pstring(" + Utils.serialize(value) + ")"
+    s"""Fixed${width.serialize}$pstring("h${value.toString(16)}")"""
   }
   def mapExpr(f: Expression => Expression): Expression = this
   def mapType(f: Type => Type): Expression = this
diff --git a/src/main/scala/firrtl/passes/CheckWidths.scala b/src/main/scala/firrtl/passes/CheckWidths.scala
index f9166a6f..4b0b1c0d 100644
--- a/src/main/scala/firrtl/passes/CheckWidths.scala
+++ b/src/main/scala/firrtl/passes/CheckWidths.scala
@@ -16,7 +16,7 @@ object CheckWidths extends Pass {
   class UninferredWidth (info: Info, mname: String) extends PassException(
     s"$info : [module $mname]  Uninferred width.")
   class WidthTooSmall(info: Info, mname: String, b: BigInt) extends PassException(
-    s"$info : [module $mname]  Width too small for constant ${serialize(b)}.")
+    s"$info : [module $mname]  Width too small for constant $b.")
   class WidthTooBig(info: Info, mname: String, b: BigInt) extends PassException(
     s"$info : [module $mname]  Width $b greater than max allowed width of $MaxWidth bits")
   class DshlTooBig(info: Info, mname: String) extends PassException(
diff --git a/src/test/scala/firrtlTests/UnitTests.scala b/src/test/scala/firrtlTests/UnitTests.scala
index 3cf25d3a..0d5d098c 100644
--- a/src/test/scala/firrtlTests/UnitTests.scala
+++ b/src/test/scala/firrtlTests/UnitTests.scala
@@ -304,7 +304,6 @@ class UnitTests extends FirrtlFlatSpec {
         (c: Circuit, p: Pass) => p.run(c)
       }
     }
-
   }
 
   "Conditional conection of clocks" should "throw an exception" in {
@@ -323,4 +322,55 @@ class UnitTests extends FirrtlFlatSpec {
       compileToVerilog(input)
     }
   }
+
+  "Parsing SInts" should "work" in {
+    val passes = Seq()
+    val input =
+      """circuit Unit :
+        |  module Unit :
+        |    node posSInt = SInt(13)
+        |    node negSInt = SInt(-13)
+        |    node posSIntString = SInt("h0d")
+        |    node posSIntString2 = SInt("h+d")
+        |    node posSIntString3 = SInt("hd")
+        |    node negSIntString = SInt("h-d")
+        |    node negSIntStringWidth = SInt<5>("h-d")
+        |    node neg2 = SInt("h-2")
+        |    node pos2 = SInt("h2")
+        |    node neg1 = SInt("h-1")
+        |""".stripMargin
+    val expected = Seq(
+      """node posSInt = SInt<5>("hd")""",
+      """node negSInt = SInt<5>("h-d")""",
+      """node posSIntString = SInt<5>("hd")""",
+      """node posSIntString2 = SInt<5>("hd")""",
+      """node posSIntString3 = SInt<5>("hd")""",
+      """node negSIntString = SInt<5>("h-d")""",
+      """node negSIntStringWidth = SInt<5>("h-d")""",
+      """node neg2 = SInt<2>("h-2")""",
+      """node pos2 = SInt<3>("h2")""",
+      """node neg1 = SInt<1>("h-1")"""
+    )
+    executeTest(input, expected, passes)
+  }
+  "Verilog SInts" should "work" in {
+    val passes = Seq()
+    val input =
+      """circuit Unit :
+        |  module Unit :
+        |    output posSInt : SInt
+        |    output negSInt : SInt
+        |    posSInt <= SInt(13)
+        |    negSInt <= SInt(-13)
+        |""".stripMargin
+    val expected = Seq(
+      """assign posSInt = 5'shd;""",
+      """assign negSInt = -5'shd;"""
+    )
+    val out = compileToVerilog(input)
+    val lines = out.split("\n") map normalized
+    expected foreach { e =>
+      lines should contain(e)
+    }
+  }
 }