Add recursive Bundle and generic Data equality (===)

[jared-barocsi]

Contributor Checklist

• Did you add Scaladoc to every public function/method?
• Did you add at least one test demonstrating the PR?
• Did you delete any extraneous printlns/debugging code?
• Did you specify the type of improvement?
• Did you add appropriate documentation in docs/src?
• Did you state the API impact?
• Did you specify the code generation impact?
• Did you request a desired merge strategy?
• Did you add text to be included in the Release Notes for this change?

Type of Improvement

• new feature/API

API Impact

• Adds recursive === operator to Data types

Backend Code Generation Impact

No impact

Desired Merge Strategy

Squash and merge

Release Notes

Implement recursive Bundle and Data equality

• This allows Bundle and Data objects to be compared with === directly, without any additional steps like casting to UInt

Reviewer Checklist (only modified by reviewer)

• Did you add the appropriate labels?
• Did you mark the proper milestone (Bug fix: 3.4.x, [small] API extension: 3.5.x, API modification or big change: 3.6.0)?
• Did you review?
• Did you check whether all relevant Contributor checkboxes have been checked?
• Did you do one of the following when ready to merge:
  • Squash: You/ the contributor Enable auto-merge (squash), clean up the commit message, and label with Please Merge.
  • Merge: Ensure that contributor has cleaned up their commit history, then merge with Create a merge commit.

[ekiwi]

I don't think two different calls to DontCare should be equal. That is not what happens in the firrtl if you did not check in the frontend.

[ekiwi]

I think comparing DontCare should not be possible. We do not want these two versions of code to (silently) have different behavior:

DontCare === DontCare

val a = Wire(UInt(4.W))
val b = Wire(UInt(4.W))
a := DontCare
b := DontCare
a === b

[jackkoenig]

We do not want these two versions of code to (silently) have different behavior

Absolutely agreed, but the behavior of the two actually is the same, both will give true. Here's a Scastie illustrating the behavior:

import chisel3._
import chisel3.util.Valid
import chisel3.experimental.BundleLiterals._

class Example extends Module {

  val out1, out2      = IO(Output(Bool()))

  val u1 = WireInit(UInt(8.W), DontCare)
  val u2 = WireInit(UInt(8.W), DontCare)
  out1 := u1 === u2
  
  val b1 = Valid(UInt(8.W)).Lit(_.bits -> 123.U) // unspecified is implicitly DontCare
  val b2 = Valid(UInt(8.W)).Lit(_.bits -> 123.U)
  out2 := b1.asUInt === b2.asUInt
}

This gives the following FIRRTL

circuit Example :
  module Example :
    input clock : Clock
    input reset : UInt<1>
    output out1 : UInt<1>
    output out2 : UInt<1>

    wire u1 : UInt<8>
    u1 is invalid
    wire u2 : UInt<8>
    u2 is invalid
    node _out1_T = eq(u1, u2)
    out1 <= _out1_T
    wire _out2_WIRE : UInt<1>
    _out2_WIRE is invalid
    node _out2_T = cat(_out2_WIRE, UInt<7>("h7b"))
    wire _out2_WIRE_1 : UInt<1>
    _out2_WIRE_1 is invalid
    node _out2_T_1 = cat(_out2_WIRE_1, UInt<7>("h7b"))
    node _out2_T_2 = eq(_out2_T, _out2_T_1)
    out2 <= _out2_T_2

And the following Verilog:

module Example(
  input   clock,
  input   reset,
  output  out1,
  output  out2
);
  assign out1 = 1'h1; // @[main.scala 13:14]
  assign out2 = 1'h1; // @[main.scala 17:21]
endmodule

[jackkoenig]

I'm not sure this is a good thing, but it is consistent.

[seldridge]

As annoying as it is, comparing two invalidated wires should return true. A lot of Chisel code intentionally or unintentionally assumes that invalid is zero in certain contexts. This is documented in CIRCT. I would love to change this, but it would be very difficult at this point. I do think defining a new concept of poison and seeing where that goes is tractable.

[jackkoenig]

I do think defining a new concept of poison and seeing where that goes is tractable.

But beyond the scope of this PR for sure 🙂

[ekiwi]

The ambiguity around DontCare === 0.U is why I feel like making them uncomparable by generating an elaboration time error might be the best solution here.

[jackkoenig]

I agree that the ambiguity is a problem, but is it a better outcome that the following would fail:

val b1 = Valid(UInt(8.W)).Lit(_.bits -> 123.U) // unspecified is implicitly DontCare
val b2 = Valid(UInt(8.W)).Lit(_.bits -> 123.U)
b1 === b2

But this would not?

val b1 = Valid(UInt(8.W)).Lit(_.bits -> 123.U) // unspecified is implicitly DontCare
val b2 = Valid(UInt(8.W)).Lit(_.bits -> 123.U)
WireInit(b1) === WireInit(b2)