Fix up Absolute value #abs (#491)

* Fix up Absolute value #abs
Defines #abs in Num
Implement #abs in UInt
Change #abs in SInt to return an SInt
Change #abs in FixedPoint to return a FixedPoint
Added  a couple of tests
Add some scala style suppression to Bits so I can read code in IntelliJ

* Per review
Add tests that abs works for positive values
Added SInt and UInt tests for abs to new underpopulated IntegerMathSpec
Used fixed point literals in fixed points abs definition

3 files changed, 73 insertions, 23 deletions

diff --git a/chiselFrontend/src/main/scala/chisel3/core/Bits.scala b/chiselFrontend/src/main/scala/chisel3/core/Bits.scala
index e885f1ee..96ea137f 100644
--- a/chiselFrontend/src/main/scala/chisel3/core/Bits.scala
+++ b/chiselFrontend/src/main/scala/chisel3/core/Bits.scala
@@ -13,6 +13,8 @@ import chisel3.internal.firrtl.PrimOp._
 // TODO: remove this once we have CompileOptions threaded through the macro system.
 import chisel3.core.ExplicitCompileOptions.NotStrict
 
+//scalastyle:off method.name
+
 /** Element is a leaf data type: it cannot contain other Data objects. Example
   * uses are for representing primitive data types, like integers and bits.
   */
@@ -48,6 +50,7 @@ abstract class Element(private[core] val width: Width) extends Data {
 /** A data type for values represented by a single bitvector. Provides basic
   * bitwise operations.
   */
+//scalastyle:off number.of.methods
 sealed abstract class Bits(width: Width, override val litArg: Option[LitArg])
     extends Element(width) {
   // TODO: perhaps make this concrete?
@@ -301,6 +304,7 @@ sealed abstract class Bits(width: Width, override val litArg: Option[LitArg])
   * types.
   */
 abstract trait Num[T <: Data] {
+  self: Num[T] =>
   // def << (b: T): T
   // def >> (b: T): T
   //def unary_-(): T
@@ -367,6 +371,10 @@ abstract trait Num[T <: Data] {
 
   def do_>= (that: T)(implicit sourceInfo: SourceInfo): Bool
 
+  /** Outputs the absolute value of `this`. The resulting width is the unchanged */
+  final def abs(): T = macro SourceInfoTransform.noArg
+  def do_abs(implicit sourceInfo: SourceInfo): T
+
   /** Outputs the minimum of `this` and `b`. The resulting width is the max of
     * the operands.
     */
@@ -431,6 +439,9 @@ sealed class UInt private[core] (width: Width, lit: Option[ULit] = None)
   final def | (that: UInt): UInt = macro SourceInfoTransform.thatArg
   final def ^ (that: UInt): UInt = macro SourceInfoTransform.thatArg
 
+//  override def abs: UInt = macro SourceInfoTransform.noArg
+  def do_abs(implicit sourceInfo: SourceInfo): UInt = this
+
   def do_& (that: UInt)(implicit sourceInfo: SourceInfo): UInt =
     binop(sourceInfo, UInt(this.width max that.width), BitAndOp, that)
   def do_| (that: UInt)(implicit sourceInfo: SourceInfo): UInt =
@@ -629,9 +640,9 @@ sealed class SInt private[core] (width: Width, lit: Option[SLit] = None)
   def do_=/= (that: SInt)(implicit sourceInfo: SourceInfo): Bool = compop(sourceInfo, NotEqualOp, that)
   def do_=== (that: SInt)(implicit sourceInfo: SourceInfo): Bool = compop(sourceInfo, EqualOp, that)
 
-  final def abs(): UInt = macro SourceInfoTransform.noArg
+//  final def abs(): UInt = macro SourceInfoTransform.noArg
 
-  def do_abs(implicit sourceInfo: SourceInfo): UInt = Mux(this < 0.S, (-this).asUInt, this.asUInt)
+  def do_abs(implicit sourceInfo: SourceInfo): SInt = Mux(this < 0.S, (-this), this)
 
   override def do_<< (that: Int)(implicit sourceInfo: SourceInfo): SInt =
     binop(sourceInfo, SInt(this.width + that), ShiftLeftOp, that)
@@ -905,10 +916,8 @@ sealed class FixedPoint private (width: Width, val binaryPoint: BinaryPoint, lit
   def do_=/= (that: FixedPoint)(implicit sourceInfo: SourceInfo): Bool = compop(sourceInfo, NotEqualOp, that)
   def do_=== (that: FixedPoint)(implicit sourceInfo: SourceInfo): Bool = compop(sourceInfo, EqualOp, that)
 
-  final def abs(): UInt = macro SourceInfoTransform.noArg
-
-  def do_abs(implicit sourceInfo: SourceInfo): UInt = {
-    Mux(this < FixedPoint.fromBigInt(0), (FixedPoint.fromBigInt(0)-this).asUInt, this.asUInt)
+  def do_abs(implicit sourceInfo: SourceInfo): FixedPoint = {
+    Mux(this < 0.F(0), 0.F(0) - this, this)
   }
 
   override def do_<< (that: Int)(implicit sourceInfo: SourceInfo): FixedPoint =
diff --git a/src/test/scala/chiselTests/FixedPointSpec.scala b/src/test/scala/chiselTests/FixedPointSpec.scala
index bfbb4e46..d0557e66 100644
--- a/src/test/scala/chiselTests/FixedPointSpec.scala
+++ b/src/test/scala/chiselTests/FixedPointSpec.scala
@@ -21,23 +21,31 @@ class FixedPointLiteralSpec extends FlatSpec with Matchers {
 }
 
 class FixedPointFromBitsTester extends BasicTester {
-    val uint = 3.U(4.W)
-    val sint = -3.S
-    val fp   = FixedPoint.fromDouble(3.0, width = 4, binaryPoint = 0)
-    val fp_tpe = FixedPoint(4.W, 1.BP)
-    val uint_result = FixedPoint.fromDouble(1.5, width = 4, binaryPoint = 1)
-    val sint_result = FixedPoint.fromDouble(-1.5, width = 4, binaryPoint = 1)
-    val fp_result   = FixedPoint.fromDouble(1.5, width = 4, binaryPoint = 1)
-
-    val uint2fp = fp_tpe.fromBits(uint)
-    val sint2fp = fp_tpe.fromBits(sint)
-    val fp2fp   = fp_tpe.fromBits(fp)
-
-    assert(uint2fp === uint_result)
-    assert(sint2fp === sint_result)
-    assert(fp2fp   === fp_result)
-
-    stop()
+  val uint = 3.U(4.W)
+  val sint = -3.S
+
+  val fp   = FixedPoint.fromDouble(3.0, width = 4, binaryPoint = 0)
+  val fp_tpe = FixedPoint(4.W, 1.BP)
+  val uint_result = FixedPoint.fromDouble(1.5, width = 4, binaryPoint = 1)
+  val sint_result = FixedPoint.fromDouble(-1.5, width = 4, binaryPoint = 1)
+  val fp_result   = FixedPoint.fromDouble(1.5, width = 4, binaryPoint = 1)
+
+  val uint2fp = fp_tpe.fromBits(uint)
+  val sint2fp = fp_tpe.fromBits(sint)
+  val fp2fp   = fp_tpe.fromBits(fp)
+
+  val negativefp = -3.5.F(binaryPoint = 4)
+  val positivefp = 3.5.F(binaryPoint = 4)
+
+  assert(uint2fp === uint_result)
+  assert(sint2fp === sint_result)
+  assert(fp2fp   === fp_result)
+
+  assert(positivefp.abs() === positivefp)
+  assert(negativefp.abs() === positivefp)
+  assert(negativefp.abs() =/= negativefp)
+
+  stop()
 }
 
 class SBP extends Module {
diff --git a/src/test/scala/chiselTests/IntegerMathSpec.scala b/src/test/scala/chiselTests/IntegerMathSpec.scala
new file mode 100644
index 00000000..3ccade8e
--- /dev/null
+++ b/src/test/scala/chiselTests/IntegerMathSpec.scala
@@ -0,0 +1,33 @@
+// See LICENSE for license details.
+
+package chiselTests
+
+import chisel3._
+import chisel3.testers.BasicTester
+
+//scalastyle:off magic.number
+class IntegerMathTester extends BasicTester {
+
+  //TODO: Add more operators
+
+  /* absolute values tests */
+
+  val uint = 3.U(4.W)
+  val sint = (-3).S()
+  val sintpos = 3.S()
+  val wrongSIntPos = 4.S()
+
+  assert(uint.abs() === uint)
+  assert(sint.abs() === sintpos)
+  assert(sintpos.abs() === sintpos)
+
+  assert(sint.abs() =/= wrongSIntPos)
+
+  stop()
+}
+
+class IntegerMathSpec extends ChiselPropSpec {
+  property("All integer ops should return the correct result") {
+    assertTesterPasses{ new IntegerMathTester }
+  }
+}