chisel icon indicating copy to clipboard operation
chisel copied to clipboard

Add `decodeAs` API to output decoded signals directly to a Bundle

Open carlosedp opened this issue 3 years ago • 9 comments

Contributor Checklist

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

Type of Improvement

  • new feature/API

API Impact

This PR adds a new decodeAs function to decoder allowing the signal decode output to go directly to a Bundle.

Backend Code Generation Impact

No Verilog output impact.

Desired Merge Strategy

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

Release Notes

Added a new decodeAs function to decoder allowing the signal decode output to go directly to a Bundle.

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 avatar Jan 06 '22 13:01 carlosedp

Thanks carlos! Could you add a example/test please?

ekiwi avatar Jan 06 '22 18:01 ekiwi

Sure, will add to the PR and below:

  class DecType extends Bundle {
    val inst_type = UInt(InstructionType.getWidth.W)
    val inst      = UInt(Instruction.getWidth.W)
    val to_alu    = Bool()
    val branch    = Bool()
    val use_imm   = Bool()
    val jump      = Bool()
    val is_load   = Bool()
    val is_store  = Bool()
  }
  val signals = decode.decodeAs(
    (new DecType),
    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) ## BitPat(ADD)    ## BitPat.Y() ## BitPat.N() ## BitPat.N() ## BitPat.N() ## BitPat.N() ## BitPat.N(),
        BitPat("b?????????????????000?????0010011")  -> BitPat(INST_I) ## BitPat(ADDI)   ## BitPat.Y() ## BitPat.N() ## BitPat.Y() ## BitPat.N() ## BitPat.N() ## BitPat.N(),
      ),                                                BitPat(IN_ERR) ## BitPat(ERR)    ## BitPat.dontCare(6) // Default values
      // format: on
    )
  )

@ekiwi so per your comment on the other PR, having (new DecType) is the correct way right?

carlosedp avatar Jan 06 '22 18:01 carlosedp

I'm trying to write a test case but it complains it's not in a Builder context:

class DecoderSpec extends AnyFlatSpec {
  "decoder" should "decodeAs to an existing bundle" in {
    class OutputBundle extends Bundle {
      val s1 = UInt(2.W)
      val s2 = UInt(2.W)
      val s3 = Bool()
    }
    val selector = "b001".U
    val signals = decodeAs(
      (new OutputBundle),
      selector,
      TruthTable(
        Array(
        BitPat("b001")  -> BitPat(1.U(2.W)) ## BitPat(1.U(2.W))   ## BitPat.Y(),
        BitPat("b?11")  -> BitPat(2.U(2.W)) ## BitPat(2.U(2.W))   ## BitPat.N(),
        ),                 BitPat(0.U(2.W)) ## BitPat(0.U(2.W))   ## BitPat.dontCare(1) // Default values
      )
    )

    assert(signals.s1 === true)
    assert(signals.s2 === true)
    assert(signals.s3 === true)
  }
}

Cc. @sequencer

carlosedp avatar Jan 06 '22 19:01 carlosedp

I'm trying to write a test case but it complains it's not in a Builder context:

You need to put your code into a chisel Module. Unfortunately most chisel constructs are only supported in that context.

ekiwi avatar Jan 06 '22 19:01 ekiwi

Added a test spec for it!

carlosedp avatar Jan 06 '22 20:01 carlosedp

In cases where my Bundle has Enums, I'm getting a cast warning like Decoder.scala:44: Casting non-literal UInt to chiselv.Instruction. You can use chiselv.Instruction.safe to cast without this warning. in class chiselv.Decoder.

I believe in the return of decodeAs, I might need to go thru each element and if it's an Enum, I need to Type.safe it. Right?

My Bundle:

  class DecType extends Bundle {
    val inst_type = InstructionType()
    val inst      = Instruction()
    val to_alu   = Bool()
    val branch   = Bool()
    val use_imm  = Bool()
    val jump     = Bool()
    val is_load  = Bool()
    val is_store = Bool()
  }

Where inst and inst_type are ChiselEnum and ChiselEnum1H.

carlosedp avatar Jan 10 '22 14:01 carlosedp

I'd like to have provide a BundleLit API for this usage. Working on a small example.

sequencer avatar Jan 10 '22 17:01 sequencer

I think this is the last missing piece to have a nice API for decode (1H enum + Mux1H + decodeAs + Bitpat accepting enums) :)

carlosedp avatar Jan 10 '22 17:01 carlosedp

I agree, Mux1H taking enum is a necessary feature that we have benign missing. As for decodeAs or similar API, I'd like have a API like this:

                                                    instr, term, default
def decode[T <: Data](input: UInt, truthTable: (Seq[(BitPat, T)], T)): T

However the BundleLiterals doesn't support BitPat(since it is not a Data). I'm trying to give a simple work around.

sequencer avatar Jan 10 '22 17:01 sequencer

Happy new year, and hopefully resolved in #2924

sequencer avatar Jan 10 '23 09:01 sequencer