add combStage() to rename returned Stream

[pwang7]

No description provided.

[kleinai]

What's the rationale for this?

Also it's best not to reformat a bunch of code you're not adding anything to. It makes the diff much larger than it needs to be.

[Dolu1990]

Is it about preserving as much as possible naming ?

[pwang7]

What's the rationale for this?

Also it's best not to reformat a bunch of code you're not adding anything to. It makes the diff much larger than it needs to be.

Sorry this PR is not done yet.
It's for renaming the returned Stream from the Stream utility functions, such as:

val myStreamName = StreamMux(...) // the generated Verilog use myStreamName as prefix

[pwang7]

Is it about preserving as much as possible naming ?

Yes.
I'm not sure whether this PR covers all the Stream utility function that returned Streams can be renamed?

[Dolu1990]

Yes.
I'm not sure whether this PR covers all the Stream utility function that returned Streams can be renamed?

Normaly it should be possible.

But one question is :

1. Do we want both a composite named stream + a returned stream for each function
   Ex def translateWith[T2 <: Data](that: T2): Stream[T2] = { in your pr
2. Else, is it good enough if the function return the composite Stream, allowing for a val xx = myStreamFunc to rename it to xx

So far, i was more for 2) in order to avoid generating "duplicated" signals"

[pwang7]

Yes.
I'm not sure whether this PR covers all the Stream utility function that returned Streams can be renamed?

Normaly it should be possible.

But one question is :

1. Do we want both a composite named stream + a returned stream for each function
   Ex def translateWith[T2 <: Data](that: T2): Stream[T2] = { in your pr
2. Else, is it good enough if the function return the composite Stream, allowing for a val xx = myStreamFunc to rename it to xx

So far, i was more for 2) in order to avoid generating "duplicated" signals"

What about this case:

val someQueue = StreamFifo(...)
val queuePop = someQueue.io.pop

Will 2) keep the name prefix of queuePop instead of someQueue_io_pop?

By stick to the name explicitly defined, it's easier to debug. But like you mentioned, it might have duplicate signals.

[Dolu1990]

So, the case of the

val someQueue = StreamFifo(...)
val queuePop = someQueue.io.pop

Is one where a .combStage is good to do, as the someQueue.io.pop is already strongly named by the fifo (will not be renamed by futher val xxx = yyy
(so it keep someQueue_io_pop)

[pwang7]

So, the case of the

val someQueue = StreamFifo(...)
val queuePop = someQueue.io.pop

Is one where a .combStage is good to do, as the someQueue.io.pop is already strongly named by the fifo (will not be renamed by futher val xxx = yyy (so it keep someQueue_io_pop)

OK, let me update my PR.

[numero-744]

Hi @pwang7

What is the status of this PR?

[Dolu1990]

Hmm i think when there is patterns like :

def xxx() : Stream[T] = {
   val ret = Stream(T())
   ...
   ret  //Should not use combStage on ret, as that Stream can be named by val zzz = stream.xxx()
}

For all things where the returned value is a stream from a sub component, then yes, combStage is 100 % good. (i think)

[numero-744]

@Dolu1990 I don't understand, what should we do with this PR?

[Dolu1990]

@numero-744 Keep the .combStage() when it wrap a subcomponent io, but remove the .combStage() when it wrap stuff from the current component (ex in an Area / Composable)

Ex good :
https://github.com/SpinalHDL/SpinalHDL/pull/749/files#diff-743af7b9a3e4fd462a17d10152d625583ad21d76cd66a9202e3bd41d84a39d7fR182

Ex to remove :
https://github.com/SpinalHDL/SpinalHDL/pull/749/files#diff-743af7b9a3e4fd462a17d10152d625583ad21d76cd66a9202e3bd41d84a39d7fR274

Also, i think the following is broken :
https://github.com/SpinalHDL/SpinalHDL/pull/749/files#diff-743af7b9a3e4fd462a17d10152d625583ad21d76cd66a9202e3bd41d84a39d7fR297

[numero-744]

Ping @pwang7

[pwang7]

Ping @pwang7

Let me fix this PR later.