Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert removal of bit extraction const prop for literals #857

Merged
merged 3 commits into from
Jul 31, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions chiselFrontend/src/main/scala/chisel3/core/Bits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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?
Expand Down
18 changes: 18 additions & 0 deletions src/test/scala/chiselTests/FixedPointSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand All @@ -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)
}
}
19 changes: 19 additions & 0 deletions src/test/scala/chiselTests/SIntOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand All @@ -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)
}

}
19 changes: 19 additions & 0 deletions src/test/scala/chiselTests/UIntOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
}