Skip to content

Conversation

sequencer
Copy link
Member

@sequencer sequencer commented Jul 19, 2024

Two bugs are illustrated from this code:

  • Instantiate failed to cache;
  • Probe and OM doesn't work well with FlatIO.

Minimal reproducible code:

    class Baz extends Bundle {
      val ele = Bool()
    }
    @instantiable
    class FooOM extends properties.Class {
      @public
      val bar = IO(Output(Property[Seq[AnyClassType]]()))
      @public
      val barIn = IO(Input(Property[Seq[AnyClassType]]()))
      bar := barIn
    }
    class FooIO extends Bundle {
      val probeA = Output(Vec(2, Probe(new Baz)))
      val outputA = Output(Vec(2, new Baz))
      val om = Output(Property[AnyClassType]())
    }
    @instantiable
    class Foo extends RawModule {
      @public
      val io = FlatIO(new FooIO)
      val bars = Seq.fill(2)(chisel3.experimental.hierarchy.Instantiate(new Bar))
      val fooOM: Instance[FooOM] = Instantiate(new FooOM)
      fooOM.barIn := Property(bars.map(_.io.om.asAnyClassType))
      bars.zipWithIndex.foreach {
        case (bar, idx) =>
          define(io.probeA(idx), bar.io.probeA)
          io.outputA(idx) := bar.io.outputA
      }
      io.om := fooOM.getPropertyReference.asAnyClassType
    }
    @instantiable
    class BarOM extends properties.Class {
      @public
      val i = IO(Input(Property[BigInt]()))
      @public
      val o = IO(Output(Property[BigInt]()))
      o := i
    }

    class BarIO extends Bundle {
      val probeA = Output(Probe(new Baz))
      val outputA = Output(new Baz)
      val om = Output(Property[AnyClassType]())
    }
    @instantiable
    class Bar extends RawModule {
      @public
      val io = FlatIO(new BarIO)
      val bazWire = 0.U.asTypeOf(new Baz)

      define(io.probeA, ProbeValue(bazWire))
      io.outputA := bazWire
      val barOM: Instance[BarOM] = Instantiate(new BarOM)
      barOM.i := Property(1)
      io.om := barOM.getPropertyReference.asAnyClassType
    }

Current result:

FIRRTL version 4.0.0
circuit Foo :
  class BarOM :
    input i : Integer
    output o : Integer

    propassign o, i

  module Bar :
    output probeA : Probe<{ ele : UInt<1>}>
    output outputA : { ele : UInt<1>}
    output om : AnyRef

    wire lit_probe_val : { ele : UInt<1>}
    connect lit_probe_val.ele, UInt<1>(0h0)
    define view_1.probeA = probe(lit_probe_val)
    connect outputA.ele, UInt<1>(0h0)
    object barOM of BarOM
    propassign barOM.i, Integer(1)
    propassign om, barOM

  class BarOM_1 :
    input i : Integer
    output o : Integer

    propassign o, i

  module Bar_1 :
    output probeA : Probe<{ ele : UInt<1>}>
    output outputA : { ele : UInt<1>}
    output om : AnyRef

    wire lit_probe_val : { ele : UInt<1>}
    connect lit_probe_val.ele, UInt<1>(0h0)
    define view_2.probeA = probe(lit_probe_val)
    connect outputA.ele, UInt<1>(0h0)
    object barOM of BarOM_1
    propassign barOM.i, Integer(1)
    propassign om, barOM

  class FooOM :
    output bar : List<AnyRef>
    input barIn : List<AnyRef>

    propassign bar, barIn

  public module Foo :
    output probeA : Probe<{ ele : UInt<1>}>[2]
    output outputA : { ele : UInt<1>}[2]
    output om : AnyRef

    inst bars_0 of Bar
    inst bars_1 of Bar_1
    object fooOM of FooOM
    propassign fooOM.barIn, List<AnyRef>(view_3.om, view_4.om)
    define view.probeA[0] = bars_0.probeA
    connect outputA[0], bars_0.outputA
    define view.probeA[1] = bars_1.probeA
    connect outputA[1], bars_1.outputA
    propassign om, fooOM

I'm not familiar with the DataView codebase... cc @jackkoenig for taking over.

Desired result:

22,40d21
<   class BarOM_1 :
<     input i : Integer
<     output o : Integer
<
<     propassign o, i
<
<   module Bar_1 :
<     output probeA : Probe<{ ele : UInt<1>}>
<     output outputA : { ele : UInt<1>}
<     output om : AnyRef
<
<     wire lit_probe_val : { ele : UInt<1>}
<     connect lit_probe_val.ele, UInt<1>(0h0)
<     define view_2.probeA = probe(lit_probe_val)
<     connect outputA.ele, UInt<1>(0h0)
<     object barOM of BarOM_1
<     propassign barOM.i, Integer(1)
<     propassign om, barOM
<
53,56c34,37
<     inst bars_1 of Bar_1
<     object fooOM of FooOM
<     propassign fooOM.barIn, List<AnyRef>(view_3.om, view_4.om)
<     define view.probeA[0] = bars_0.probeA
---
>     inst bars1 of Bar
>     object fooOM f FooOM
>     propassign fooOM.barIn,List<AnyRef>(bars_0.om, bars_1.om)
>     define probeA[0] = bars_0.probeA
58c39
<     define view.probeA[1] = bars_1.probeA
---
>     define probeA[1] = bars_1.probeA

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 request a desired merge strategy?
  • Did you add text to be included in the Release Notes for this change?

Type of Improvement

  • Feature (or new API)
  • API modification
  • API deprecation
  • Backend code generation
  • Performance improvement
  • Bugfix
  • Documentation or website-related
  • Dependency update
  • Internal or build-related (includes code refactoring/cleanup)

Desired Merge Strategy

  • Squash: The PR will be squashed and merged (choose this if you have no preference).
  • Rebase: You will rebase the PR onto master and it will be merged with a merge commit.

Release Notes

Reviewer Checklist (only modified by reviewer)

  • Did you add the appropriate labels? (Select the most appropriate one based on the "Type of Improvement")
  • Did you mark the proper milestone (Bug fix: 3.6.x, 5.x, or 6.x depending on impact, API modification or big change: 7.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.

@sequencer sequencer added the Bugfix Fixes a bug, will be included in release notes label Jul 19, 2024
@sequencer sequencer changed the title Fix Probe Bundle + FixedIOModule Bug Probe and Property doest work with Probe and Property Jul 20, 2024
@sequencer sequencer changed the title Probe and Property doest work with Probe and Property Probe and Property doest work with FlatIO(DataView) Jul 20, 2024
@jackkoenig
Copy link
Contributor

Fixed by #4308 + #4309

@jackkoenig jackkoenig closed this Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix Fixes a bug, will be included in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants