Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add StreamUtils #5

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add StreamUtils #5

wants to merge 4 commits into from

Conversation

BIGWJZ
Copy link
Collaborator

@BIGWJZ BIGWJZ commented Jul 15, 2024

StreamUtils includes 2 modules: mkStreamConcat and mkStreamSplit

src/StreamUtils.bsv Outdated Show resolved Hide resolved
src/StreamUtils.bsv Outdated Show resolved Hide resolved
src/StreamUtils.bsv Outdated Show resolved Hide resolved
src/StreamUtils.bsv Outdated Show resolved Hide resolved
src/StreamUtils.bsv Outdated Show resolved Hide resolved
test/TestStreamUtils.bsv Show resolved Hide resolved
src/StreamUtils.bsv Outdated Show resolved Hide resolved
isStreamAEnd <= False;
end
// the last StreamA + the first StreamB
else if (streamA.isLast && prepareFifoB.notEmpty) begin
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

infact, the condition in this else if statement is not necessary. the bsv has implicit conditions, so if the prepareFifoB is empty, this else if branch will not be executed.

but you write the condition here is good for code readability, you can keep the condition here, but this lead to another problem: this if statement has other possiblity that is not covered. you should add a else branch, and add some code there, maybe only some comment like "in all other condition, wait for stream B to be ready"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the condition here is not necessary but good for understanding. I have added a else branch where just waiting for streamB in this clock.

hasLastRemainReg <= streamB.isLast;
remainStreamReg <= remainStream;
remainBytePtrReg <= remainBytePtr;
isStreamAEnd <= !streamB.isLast;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isStreamAEnd is misleading. isStreamAEnd looks like streamA.isLast, but infact it's not.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isStreamAEnd means streamA.isLast, but it need to be reset when the whole concat ends, i.e. streamB.isLast asserts. The codes here are misleading and I have modified for better understanding.

test StreamShift
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants