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

[VTA] [Chisel] fix tensor issue/commit in gemm #3637

Merged
merged 2 commits into from
Jul 27, 2019
Merged

[VTA] [Chisel] fix tensor issue/commit in gemm #3637

merged 2 commits into from
Jul 27, 2019

Conversation

vegaluisjose
Copy link
Member

@vegaluisjose vegaluisjose commented Jul 27, 2019

Background
The inflight counter tracks the number of inp and wgt tensors issue to the MatrixVectorMultiplication module.

Bug
The current bug is related to the way the counter is being incremented. The counter is incremented on the same state sExe where the GEMM unit asserts the done signal. Since the increment happen on the next cycle then the unit will assert done but the values are not written back to memory yet because of the depth of the pipeline.

Solution
Now, we increment the inflight counter once we read the tensors and decrement it when we write-back.

@@ -39,10 +39,10 @@ class MAC(aBits: Int = 8, bBits: Int = 8, cBits: Int = 16) extends Module {
val rA = RegNext(io.a)
val rB = RegNext(io.b)
val rC = RegNext(io.c)

Copy link
Contributor

Choose a reason for hiding this comment

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

These edits will add noise to the github history of the file. Can we fix all of the spacing issues in a separate PR for the Chisel codebase until we have lint tests?

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually I just realized - the edit was adding a tab; the spacing issues were already fixed before. That said a lint test would have caught that

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, let me try to do that.

Btw, all the ASF headers for the entire codebase has the same issue.

@tmoreau89 tmoreau89 merged commit da40645 into apache:master Jul 27, 2019
@vegaluisjose vegaluisjose deleted the hotfix-gemm branch July 28, 2019 01:26
wweic pushed a commit to wweic/tvm that referenced this pull request Aug 9, 2019
* fix tensor issue/commit in gemm

* remove trailing spaces
wweic pushed a commit to neo-ai/tvm that referenced this pull request Sep 6, 2019
* fix tensor issue/commit in gemm

* remove trailing spaces
tqchen pushed a commit to tqchen/tvm that referenced this pull request Mar 29, 2020
* fix tensor issue/commit in gemm

* remove trailing spaces
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