CloneType Should Preserve Probe-ness

[seldridge]

The following seems like incorrect behavior. If I cloneType a Probe, I get back the original, un-probed type. It should return the Probe type.

Consider:

//> using scala "2.13.11"
//> using repository sonatype-s01:snapshots
//> using lib "org.chipsalliance::chisel::6.0.0-M3+117-2372b1c4-SNAPSHOT"
//> using plugin "org.chipsalliance:::chisel-plugin::6.0.0-M3+117-2372b1c4-SNAPSHOT"
//> using options "-unchecked", "-deprecation", "-language:reflectiveCalls", "-feature", "-Xcheckinit", "-Xfatal-warnings", "-Ywarn-dead-code", "-Ywarn-unused", "-Ymacro-annotations"

import chisel3._
import chisel3.probe.Probe
import circt.stage.ChiselStage

class Foo extends RawModule {
  val a = IO(Probe(Bool()))
  val b = IO(a.cloneType)
}

object Main extends App {
  println(
    ChiselStage.emitCHIRRTL(
      new Foo
    )
  )
}

This produces (scala-cli Foo.scala):

FIRRTL version 3.3.0
circuit Foo :
  module Foo : 
    output a : Probe<UInt<1>> 
    output b : UInt<1> 

    skip

I'd expect to get:

FIRRTL version 3.3.0
circuit Foo :
  module Foo : 
    output a : Probe<UInt<1>> 
    output b : Probe<UInt<1>>

    skip

[mwachs5]

I dont know if I agree cloneType should preserve Probe-Ness. We have all these APIs:

• cloneType: What you'd get by running the same scala constructor of the thing
• MISSING API: Same as above but copying Probe-ness and Const-ness
• cloneTypeFull: What you get with cloneType plus all the contextual modifiers (Probe, Input, Output, Flipped, Const)
• chiselTypeClone: public version of cloneTypeFull
• chiselTypeOf: same as chiselTypeClone but checks to make sure you run it on real hardware first

Can we make sure whatever docs we have about these things are aligned with moving Probe into cloneType vs in cloneTypeFull?

This is pointing to the need to make Probe a real scala type thing. We shouldn't be trying to connect T to Probe(T) anyway so I'm not sure why we want this...

[mwachs5]

Should this bug also address Const-ness? Should they be considered the same way?

[seldridge]

DataMirror.internal.chiselTypeClone or chiselTypeOf do the expected thing here for both Probe and Const modifiers. So, maybe this is a "working as intended"?

Do you recall why chiselTypeOf has the restriction of something being hardware? Because this checks for hardware-ness, it then means that chiselTypeClone has to exist.

How about this: let's refactor all these into either two public functions:

1. cloneType
2. cloneTypeWithModifiers (or cloneTypeFull, though it isn't clear what "full" means here)

Or just one public function:

1. cloneType(withModifiers: Boolean = false)

The notion of requireIsHardware is dropped from all of these and is something that a user can opt into if they want it.

[mwachs5]

I'm OK with this general approach unless we want the modifiers to be divided into "things we consider part of the type and we'd want to be in the scala type if we could" (maybe probe is that? How about const?) and the direction ones? Is there any reason to bucket them differently? I know @azidar has advocated in the past that "Stripped-ness" is different than "Flipped-ness" in this regard but I don't think I can coherently echo the argument.

[seldridge]

The FIRRTL perspective, which I think is the new Chisel perspective, is that direction is not part of the type nor is it a modifier. It's a bit associated with a port. Flip is part of the type, but can only be applied to bundle fields and is not interchangeable with direction or anything else.

[mwachs5]

Flip is part of the type, but can only be applied to bundle fields and is not interchangeable with direction or anything else.

What about Stripping operations (e.g. Output when not associated with a port). Is that part of the type like Flip is?). Keep in mind these stripping concepts don't have an analogy in firrtl, this is purely a chisel problem.

Part of this comes in when you go to foo.bar.baz.cloneTypeFull, it doesn't matter that baz was Flipped or not (or does it? I still haven't fully wrapped my head around what that is supposed to mean). But it could matter if foo, or bar, or baz was Output because that is a stripping operation. Do you want to get the stripped behavior in your cloned type or not?

I think const and probe are more like stripping in this regard -- once the parent is const or probe it 'infects' all the children. Flipped is different (maybe?)

[mwachs5]

put another way, let's run your example from the Bug using plain old Output, and Flipped, and Input with a Decoupled(Bool()). What do you expect to happen even if using cloneTypeFull? What if you call cloneTypeFull on a subfield like ready or valid?

I have never fully internalized how these things work and how we expect them to work.