diff --git a/chiselFrontend/src/main/scala/chisel3/core/Bits.scala b/chiselFrontend/src/main/scala/chisel3/core/Bits.scala index c3645700b7a..35f6c9789fc 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/Bits.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/Bits.scala @@ -130,8 +130,14 @@ sealed abstract class Bits(width: Width) if (x < 0) { Builder.error(s"Negative bit indices are illegal (got $x)") } - requireIsHardware(this, "bits to be indexed") - pushOp(DefPrim(sourceInfo, Bool(), BitsExtractOp, this.ref, ILit(x), ILit(x))) + // This preserves old behavior while a more more consistent API is under debate + // See https://github.com/freechipsproject/chisel3/issues/867 + litOption.map { value => + (((value >> x.toInt) & 1) == 1).asBool + }.getOrElse { + requireIsHardware(this, "bits to be indexed") + pushOp(DefPrim(sourceInfo, Bool(), BitsExtractOp, this.ref, ILit(x), ILit(x))) + } } /** Returns the specified bit on this wire as a [[Bool]], statically @@ -169,8 +175,14 @@ sealed abstract class Bits(width: Width) Builder.error(s"Invalid bit range ($x,$y)") } val w = x - y + 1 - requireIsHardware(this, "bits to be sliced") - pushOp(DefPrim(sourceInfo, UInt(Width(w)), BitsExtractOp, this.ref, ILit(x), ILit(y))) + // This preserves old behavior while a more more consistent API is under debate + // See https://github.com/freechipsproject/chisel3/issues/867 + litOption.map { value => + ((value >> y) & ((BigInt(1) << w) - 1)).asUInt(w.W) + }.getOrElse { + requireIsHardware(this, "bits to be sliced") + pushOp(DefPrim(sourceInfo, UInt(Width(w)), BitsExtractOp, this.ref, ILit(x), ILit(y))) + } } // REVIEW TODO: again, is this necessary? Or just have this and use implicits? diff --git a/src/test/scala/chiselTests/FixedPointSpec.scala b/src/test/scala/chiselTests/FixedPointSpec.scala index ff4b42a09fe..137ffc76bea 100644 --- a/src/test/scala/chiselTests/FixedPointSpec.scala +++ b/src/test/scala/chiselTests/FixedPointSpec.scala @@ -116,6 +116,21 @@ class SBPTester extends BasicTester { stop() } +class FixedPointLitExtractTester extends BasicTester { + assert(-4.75.F(2.BP)(1) === false.B) + assert(-4.75.F(2.BP)(2) === true.B) + assert(-4.75.F(2.BP)(100) === true.B) + assert(-4.75.F(2.BP)(3, 0) === "b1101".U) + assert(-4.75.F(2.BP)(9, 0) === "b1111101101".U) + + assert(-4.75.F(6.W, 2.BP)(1) === false.B) + assert(-4.75.F(6.W, 2.BP)(2) === true.B) + assert(-4.75.F(6.W, 2.BP)(100) === true.B) + assert(-4.75.F(6.W, 2.BP)(3, 0) === "b1101".U) + assert(-4.75.F(6.W, 2.BP)(9, 0) === "b1111101101".U) + stop() +} + class FixedPointSpec extends ChiselPropSpec { property("should allow set binary point") { assertTesterPasses { new SBPTester } @@ -129,4 +144,7 @@ class FixedPointSpec extends ChiselPropSpec { property("Negative shift amounts are invalid") { a [ChiselException] should be thrownBy { elaborate(new NegativeShift(FixedPoint(1.W, 0.BP))) } } + property("Bit extraction on literals should work for all non-negative indices") { + assertTesterPasses(new FixedPointLitExtractTester) + } } diff --git a/src/test/scala/chiselTests/SIntOps.scala b/src/test/scala/chiselTests/SIntOps.scala index 2c8267af03e..9309c915e9e 100644 --- a/src/test/scala/chiselTests/SIntOps.scala +++ b/src/test/scala/chiselTests/SIntOps.scala @@ -83,6 +83,20 @@ class SIntOpsTester(c: SIntOps) extends Tester(c) { } */ +class SIntLitExtractTester extends BasicTester { + assert(-5.S(1) === true.B) + assert(-5.S(2) === false.B) + assert(-5.S(100) === true.B) + assert(-5.S(3, 0) === "b1011".U) + assert(-5.S(9, 0) === "b1111111011".U) + assert(-5.S(4.W)(1) === true.B) + assert(-5.S(4.W)(2) === false.B) + assert(-5.S(4.W)(100) === true.B) + assert(-5.S(4.W)(3, 0) === "b1011".U) + assert(-5.S(4.W)(9, 0) === "b1111111011".U) + stop() +} + class SIntOpsSpec extends ChiselPropSpec { property("SIntOps should elaborate") { @@ -94,4 +108,9 @@ class SIntOpsSpec extends ChiselPropSpec { } ignore("SIntOpsTester should return the correct result") { } + + property("Bit extraction on literals should work for all non-negative indices") { + assertTesterPasses(new SIntLitExtractTester) + } + } diff --git a/src/test/scala/chiselTests/UIntOps.scala b/src/test/scala/chiselTests/UIntOps.scala index 149ec3c446a..d583c0bba13 100644 --- a/src/test/scala/chiselTests/UIntOps.scala +++ b/src/test/scala/chiselTests/UIntOps.scala @@ -96,6 +96,21 @@ class NegativeShift(t: => Bits) extends Module { Reg(t) >> -1 } +class UIntLitExtractTester extends BasicTester { + assert("b101010".U(2) === false.B) + assert("b101010".U(3) === true.B) + assert("b101010".U(100) === false.B) + assert("b101010".U(3, 0) === "b1010".U) + assert("b101010".U(9, 0) === "b0000101010".U) + + assert("b101010".U(6.W)(2) === false.B) + assert("b101010".U(6.W)(3) === true.B) + assert("b101010".U(6.W)(100) === false.B) + assert("b101010".U(6.W)(3, 0) === "b1010".U) + assert("b101010".U(6.W)(9, 0) === "b0000101010".U) + stop() +} + class UIntOpsSpec extends ChiselPropSpec with Matchers { // Disable shrinking on error. implicit val noShrinkListVal = Shrink[List[Int]](_ => Stream.empty) @@ -120,5 +135,9 @@ class UIntOpsSpec extends ChiselPropSpec with Matchers { property("Negative shift amounts are invalid") { a [ChiselException] should be thrownBy { elaborate(new NegativeShift(UInt())) } } + + property("Bit extraction on literals should work for all non-negative indices") { + assertTesterPasses(new UIntLitExtractTester) + } }