[Bugfix] [VTA] VTA DRAM Have A Logic Issue May Cause GEMM Output Wrong. #3278
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Symptom: after change “LOG_BLOCK_IN” and “LOG_BLOCK_OUT” from vta_config.json into 7, run vta "Simple Matrix Multiply" in "simulator", the vta calculate result for GEMM is wrong. Sometime VTA crash with error “Check failed: phy_addr != 0 (0 vs. 0) : trying to get address that is nullptr” Analysis: Simulator hardcode kPageSize into 1<<12 and physical address calculate based on this size, when doing “insn->dram_base” calculation , because GetElemBytes(dst_memory_type) larger than page size, different physcial address may get same dram_base, than caused logic issue and finally trigger GEMM out put is wrong. Solution: add logic to check if PAGE SIZE larger then "GetElemBytes" return value.
|
@tmoreau89, could you help to review patch? Thanks |
tmoreau89
requested changes
Jun 3, 2019
|
Thanks, looks good! |
tmoreau89
approved these changes
Jun 4, 2019
wweic
pushed a commit
to wweic/tvm
that referenced
this pull request
Jun 26, 2019
…g. (apache#3278) * [Bugfix] [VTA] VTA DRAM Have A Logic Issue May Cause GEMM Output Wrong. Symptom: after change “LOG_BLOCK_IN” and “LOG_BLOCK_OUT” from vta_config.json into 7, run vta "Simple Matrix Multiply" in "simulator", the vta calculate result for GEMM is wrong. Sometime VTA crash with error “Check failed: phy_addr != 0 (0 vs. 0) : trying to get address that is nullptr” Analysis: Simulator hardcode kPageSize into 1<<12 and physical address calculate based on this size, when doing “insn->dram_base” calculation , because GetElemBytes(dst_memory_type) larger than page size, different physcial address may get same dram_base, than caused logic issue and finally trigger GEMM out put is wrong. Solution: add logic to check if PAGE SIZE larger then "GetElemBytes" return value. * address review comments.
wweic
pushed a commit
to neo-ai/tvm
that referenced
this pull request
Jun 27, 2019
…g. (apache#3278) * [Bugfix] [VTA] VTA DRAM Have A Logic Issue May Cause GEMM Output Wrong. Symptom: after change “LOG_BLOCK_IN” and “LOG_BLOCK_OUT” from vta_config.json into 7, run vta "Simple Matrix Multiply" in "simulator", the vta calculate result for GEMM is wrong. Sometime VTA crash with error “Check failed: phy_addr != 0 (0 vs. 0) : trying to get address that is nullptr” Analysis: Simulator hardcode kPageSize into 1<<12 and physical address calculate based on this size, when doing “insn->dram_base” calculation , because GetElemBytes(dst_memory_type) larger than page size, different physcial address may get same dram_base, than caused logic issue and finally trigger GEMM out put is wrong. Solution: add logic to check if PAGE SIZE larger then "GetElemBytes" return value. * address review comments.
tqchen
pushed a commit
to tqchen/tvm
that referenced
this pull request
Mar 29, 2020
…g. (apache#3278) * [Bugfix] [VTA] VTA DRAM Have A Logic Issue May Cause GEMM Output Wrong. Symptom: after change “LOG_BLOCK_IN” and “LOG_BLOCK_OUT” from vta_config.json into 7, run vta "Simple Matrix Multiply" in "simulator", the vta calculate result for GEMM is wrong. Sometime VTA crash with error “Check failed: phy_addr != 0 (0 vs. 0) : trying to get address that is nullptr” Analysis: Simulator hardcode kPageSize into 1<<12 and physical address calculate based on this size, when doing “insn->dram_base” calculation , because GetElemBytes(dst_memory_type) larger than page size, different physcial address may get same dram_base, than caused logic issue and finally trigger GEMM out put is wrong. Solution: add logic to check if PAGE SIZE larger then "GetElemBytes" return value. * address review comments.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Symptom:
after change “LOG_BLOCK_IN” and “LOG_BLOCK_OUT” from vta_config.json
into 7, run vta "Simple Matrix Multiply" in "simulator", the vta
calculate result for GEMM is wrong.
Sometime VTA crash with error “Check failed: phy_addr != 0 (0 vs. 0) :
trying to get address that is nullptr”
Analysis:
Simulator hardcode kPageSize into 1<<12 and physical address calculate
based on this size, when doing “insn->dram_base” calculation , because
GetElemBytes(dst_memory_type) larger than page size, different physcial
address may get same dram_base, than caused logic issue and finally
trigger GEMM out put is wrong.
Solution:
add logic to check if PAGE SIZE larger then "GetElemBytes" return value.
Thanks for contributing to TVM! Please refer to guideline https://docs.tvm.ai/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from Reviewers.