diff options
| author | Fabian Schuiki | 2021-04-16 16:18:32 +0200 |
|---|---|---|
| committer | GitHub | 2021-04-16 14:18:32 +0000 |
| commit | e9b2946c962f91a04611e32b1a9d03f78e7edf2b (patch) | |
| tree | 20f2ff423691446e3fcfcab5ce630ab2923b3c8c /src | |
| parent | fc86112bc09f3e804d76e329fea96acb70e4909d (diff) | |
Fix signedness of xor const prop with zero (#2179)
Constant propagation of the Xor op folds `xor(a, SInt(0))` to
`asUInt(a)`. For comparison, Or folds to `asUInt(pad(a, W))`. This can
be a problem in the following case:
circuit Foo :
module Foo :
input a: UInt<3>
output b: UInt<4>
b <= asUInt(xor(asSInt(a), SInt<4>(0)))
This would emit the assignment as `b = a` instead of the sign-extended
`b = {{1{a[2]}},a}`.
This requires adjusting the `pad(e, t)` function use in const prop,
which currently just inserts a `Pad` prim op with the requested output
type. However, the function advertises that it pads *to the width* of
the type `t`. Some of the folds rely on this and request the padding of
a SInt<N> to the width of a UInt<M>. But the current implementation then
then actually returns a `Pad` op with type UInt<M>, instead of the
SInt<M> that was requested.
Diffstat (limited to 'src')
| -rw-r--r-- | src/main/scala/firrtl/transforms/ConstantPropagation.scala | 14 | ||||
| -rw-r--r-- | src/test/scala/firrtlTests/ConstantPropagationTests.scala | 7 |
2 files changed, 19 insertions, 2 deletions
diff --git a/src/main/scala/firrtl/transforms/ConstantPropagation.scala b/src/main/scala/firrtl/transforms/ConstantPropagation.scala index 5610c7e7..bc1fc9af 100644 --- a/src/main/scala/firrtl/transforms/ConstantPropagation.scala +++ b/src/main/scala/firrtl/transforms/ConstantPropagation.scala @@ -29,7 +29,17 @@ object ConstantPropagation { /** Pads e to the width of t */ def pad(e: Expression, t: Type) = (bitWidth(e.tpe), bitWidth(t)) match { - case (we, wt) if we < wt => DoPrim(Pad, Seq(e), Seq(wt), t) + case (we, wt) if we < wt => + DoPrim( + Pad, + Seq(e), + Seq(wt), + e.tpe match { + case UIntType(_) => UIntType(IntWidth(wt)) + case SIntType(_) => SIntType(IntWidth(wt)) + case _ => e.tpe + } + ) case (we, wt) if we == wt => e } @@ -252,7 +262,7 @@ class ConstantPropagation extends Transform with RegisteredTransform with Depend } def simplify(e: Expression, lhs: Literal, rhs: Expression) = lhs match { case UIntLiteral(v, _) if v == BigInt(0) => rhs - case SIntLiteral(v, _) if v == BigInt(0) => asUInt(rhs, e.tpe) + case SIntLiteral(v, _) if v == BigInt(0) => asUInt(pad(rhs, e.tpe), e.tpe) case _ => e } def matchingArgsValue(e: DoPrim, arg: Expression) = UIntLiteral(0, getWidth(arg.tpe)) diff --git a/src/test/scala/firrtlTests/ConstantPropagationTests.scala b/src/test/scala/firrtlTests/ConstantPropagationTests.scala index bc7f92e6..ababb95b 100644 --- a/src/test/scala/firrtlTests/ConstantPropagationTests.scala +++ b/src/test/scala/firrtlTests/ConstantPropagationTests.scala @@ -1531,22 +1531,29 @@ class ConstantPropagationIntegrationSpec extends LowTransformSpec { val input = s"""|circuit Foo: | module Foo: + | input in1: SInt<3> | output out1: UInt<2> | output out2: UInt<2> | output out3: UInt<2> + | output out4: UInt<4> | out1 <= xor(SInt<2>(-1), SInt<2>(1)) | out2 <= or(SInt<2>(-1), SInt<2>(1)) | out3 <= and(SInt<2>(-1), SInt<2>(-2)) + | out4 <= xor(in1, SInt<4>(0)) |""".stripMargin val check = s"""|circuit Foo: | module Foo: + | input in1: SInt<3> | output out1: UInt<2> | output out2: UInt<2> | output out3: UInt<2> + | output out4: UInt<4> | out1 <= UInt<2>(2) | out2 <= UInt<2>(3) | out3 <= UInt<2>(2) + | node _GEN_0 = pad(in1, 4) + | out4 <= asUInt(_GEN_0) |""".stripMargin execute(input, check, Seq.empty) } |
