Deprecate ListLookup.

[sequencer]

This API considers harmful and misleading to users. Most of users use this API as decoder, however this is a sequentially cascaded Mux without logic minimization, synthesizer won't aware of the ME(mutually exclusive) from this API, which provides a bad PPA for hardware designing. Real decoder should be a minimizer+PLA which has been implemented in chisel3.util.experimental.decode.

My proposal is:

1. deprecating this API in Chisel 3.5
2. moving decoder API out of experimental at the last minor version of Chisel 3.5
3. delete this API in Chisel 3.6.

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

• code cleanup

API Impact

deprecate chisel3.utils.ListLookup

Backend Code Generation Impact

None

Desired Merge Strategy

• Squash: The PR will be squashed and merged (choose this if you have no preference.

Release Notes

chisel3.util.ListLookup is replaced by chisel3.util.experimental.decode

Reviewer Checklist (only modified by reviewer)

• [ ] Did you add the appropriate labels?
• [ ] Did you mark the proper milestone (Bug fix: 3.3.x, [small] API extension: 3.4.x, API modification or big change: 3.5.0)?
• [ ] Did you review?
• [ ] Did you check whether all relevant Contributor checkboxes have been checked?
• [ ] Did you mark as Please Merge?

[carlosedp]

I believe we could have some syntactic sugar in chisel3.util.experimental.decode wrapping the functionality for an easy-to-approach way to define the decoder (as straightforward as in ListLookup).

The process of having a bundle, assigning the outputs of the decoder to it and casting to the bundle uses a complex pattern.

[sequencer]

You are right, I think I can add a something like decodeAs API to keep the type safety. And TruthTable should add a sugar to consume BundleLit to generate a UInt.

[carlosedp]

@sequencer added a nice example to my core on the use of decoder/TruthTable: https://github.com/carlosedp/chiselv/pull/2

I think there are a couple of places it could be "nicer" :)

  val signals = decode
    .decoder(
      io.DecoderPort.op,
      decode.TruthTable(
        // format: off
        Array(
        /*                                                     inst_type,                                     ##        inst                                       ##  to_alu    ##  branch    ##  use_imm   ##   jump     ##  is_load   ## is_store */
        // Arithmetic
        BitPat("b0000000??????????000?????0110011")  -> BitPat(INST_R.litValue.U(InstructionType.getWidth.W)) ## BitPat(ADD.litValue.U(Instruction.getWidth.W))    ## BitPat.Y() ## BitPat.N() ## BitPat.N() ## BitPat.N() ## BitPat.N() ## BitPat.N(),
        BitPat("b?????????????????000?????0010011")  -> BitPat(INST_I.litValue.U(InstructionType.getWidth.W)) ## BitPat(ADDI.litValue.U(Instruction.getWidth.W))   ## BitPat.Y() ## BitPat.N() ## BitPat.Y() ## BitPat.N() ## BitPat.N() ## BitPat.N(),

1. I think the pattern BitPat(INST_R.litValue.U(InstructionType.getWidth.W)) could have some kind of macro to shorten it up since the idea is pretty straightforward and can be reused in many places.
2. I'm not a huge fan of casting and the .asTypeOf(new DecType) is a bit confusing... maybe it could be improved somehow?
3. Same here io.DecoderPort.inst := signals.inst.asTypeOf(new Instruction.Type) where I had to cast the returned UInt back to the Enum.

Other than that, it even improved the clock and logic elements usage on my core.

Thanks a lot @sequencer :D

[carlosedp]

Addressed point 1 above with https://github.com/chipsalliance/chisel3/pull/2327

[carlosedp]

@sequencer I've implemented a decodeAs method like:

object decodeAs  {
    /** Decode signals using a [[Bundle]] as the output
      *
      * @param b          the output bundle to be used as the decoded signal
      * @param input      input signal that contains decode table input
      * @param truthTable [[TruthTable]] to decode user input.
      * @return bundle with the decode output.
      *
      * @note The bundle width must match the TruthTable width.
      */
    def apply[T <: Bundle](b: T, input: UInt, truthTable: TruthTable): T =
    decoder(input, truthTable).asTypeOf(b)
    def apply[T <: Bundle](minimizer: Minimizer, b: T, input: UInt, truthTable: TruthTable): T =
    decoder(minimizer, input, truthTable).asTypeOf(b)
}

But this way, I have to call it as:

  val signals = decode.decoder.decodeAs(
    (new DecType),
    io.DecoderPort.op,
    decode.TruthTable(
        Array(
...

How can I make it not require the (new DecType) and be able to accept the DecType directly?

FYI, opened https://github.com/chipsalliance/chisel3/pull/2328 to address this.

[ekiwi]

How can I make it not require the (new DecType) and be able to accept the DecType directly?

new DecType is correct. This stems from the fact that Scala type != Chisel type. The instance of the bundle is the Chisel type.

[ekiwi]

I want to say that this whole decoder thing seems to have a significant disadvantage compared to the old list lookup, namely it looks like you have to concatenate together all outputs and then manually extract the bits again (or do an unsafe cast to a Bundle). I would love for there to be a better, more type safe API. Maybe something using Bundle Literals and making sure that the output can only be of the type that was used to create the decoder? Like can we have the Scala type system enforce that you use DecType to declare the lookup table and thus the output of the decoder is also DecType?

[sequencer]

Yes, APIs for decoder is pretty primitive, I have been thinking adding a new dataview based API to view instructions as decode result. I'll prototype it this weekend(Sorry I have been too busy recently.)

[ekiwi]

Yes, APIs for decoder is pretty primitive, I have been thinking adding a new dataview based API to view instructions as decode result. I'll prototype it this weekend(Sorry I have been too busy recently.)

Awesome! Do you think you could prototype it in the context of chiselv? This way carlos can give you direct feedback.

[sequencer]

Sounds good. I'll have a try in Carlos repo.

[carlosedp]

Yep, I'm trying to piece together a good API thru the PRs below:

• #2327
• #2261
• #2328

I got them together and published locally to try on my core and got some nice code as can be seen in https://github.com/carlosedp/chiselv/blob/d676295d9e6cc16d4df973f2ea13005a775130bb/src/main/scala/Decoder.scala

[ekiwi]

I got them together and published locally to try on my core and got some nice code as can be seen in

The part that I thought was rather unfortunate is here: https://github.com/carlosedp/chiselv/blob/d676295d9e6cc16d4df973f2ea13005a775130bb/src/main/scala/Decoder.scala#L111

Having to bit-extract the result seems very error prone.

Maybe the solution could be that the decoder accepts as List[BitPat] and then returns a List[UInt] similar to how the old ListLookup seemed to work.

[sequencer]

Here is my plan: I'm adding a API like this:

implicit class RecordToBitPat[T <: Record](x: T) {
  def bp[T <: Record](elems: x.type => Map[Data, BitPat]): BitPat = ???
}

Then a Bundle/Record can use this API to construct BitPat:

object MyEnum extends ChiselEnum {
  val sA, sB = Value
}
class MyBundle extends Bundle {
  val a = UInt(8.W)
  val b = Bool()
  val c = MyEnum()
}
val someBp: BitPat = (new MyBundle).bp { b =>
  Map(
    b.a -> BitPat(5.U),
    b.b -> BitPat.N(),
    b.c -> BitPat(MyEnum.sB.litValue.U)
  )
}

With this API, Construction of TruthTable should be easier. After implementing this, adding a new API def chiselType: Data to TruthTable can provide a cloneType-like functionality, which will be called by decoder to select which ChiselType should be casted to.

[ekiwi]

Looks good to me. With one of @carlosedp's PRs, we could even simplify:

b.c -> BitPat(MyEnum.sB.litValue.U)

to

b.c -> BitPat(MyEnum.sB)

[sequencer]

Looks good to me. With one of @carlosedp's PRs, we could even simplify:

b.c -> BitPat(MyEnum.sB.litValue.U)

to

b.c -> BitPat(MyEnum.sB)

Yes I agree. BitPat and MuxOH should certainly consume the Enum as parameter for simpler API.

[carlosedp]

The part that I thought was rather unfortunate is here: carlosedp/chiselv@d676295/src/main/scala/Decoder.scala#L111

Having to bit-extract the result seems very error prone.

Hi Kev, actually I don't bit-extract from the decoder over there (it's extracting the registers from origin op only)... what I do is casting back to the Bundle here https://github.com/carlosedp/chiselv/blob/d676295d9e6cc16d4df973f2ea13005a775130bb/src/main/scala/Decoder.scala#L128-L134 using the decodeAs function I created at #2328