`hlslib::Stream::write` works like `hls::stream::write` about hlslib HOT 4 CLOSED

Hey there! This is intended: My rationale with the compatibility interface is that it changes nothing from if you had been using hls::stream, such that users would experience no difference in behavior when changing their data type. Rather, the difference comes when using the hlslib API.

I see the argument for either approach, so I'm on the fence. If it's just for the sake of the presentation, I would lean towards not changing the behavior, because it's not really the intended API anyway, so it doesn't seem worth the risk of breaking some existing code :-)

from hlslib.

You are right, we certainly shouldn't change it for the sake of a presentation! =D

That being said, I do believe we should change it so let me make my case.
I think the purpose of the dataflow macros combined with hlslib::Stream is the realistic simulation of hardware to decrease development time by catching deadlocks. In particular, hls::Stream can be used as a drop-in replacement without changing the rest of the code. As it stands however, if a user does indeed just replace hls::stream with hlslib::Stream they do not in fact get the realistic behavior - they still get an infinite FIFO, which obviously can't be put into hardware.

Our (can I call it our? :D) readme also states that

[hls::Streams are] bounded, simulating the finite capacity of hardware FIFOs, allowing easier detection of deadlocks in software

A reader would thus expect finite FIFOs, so I would argue that changing the ::write method to behave like ::Push would not break, but indeed fix existing code - paradoxically by potentially causing deadlocks.

As an additional note, I wasn't aware of this difference until I stumbled over the issue I laid out in my first post, and actually read the source code more in depth. The difference is just an additional parameter being passed to the same class method, so it is easy to miss.

from hlslib.

Okay, sure! Feel free to push a PR :-)

from hlslib.

Closing this since its merged!

from hlslib.