Skip to content

Commit

Permalink
[Bugfix] [VTA] VTA DRAM Have A Logic Issue May Cause GEMM Output Wron…
Browse files Browse the repository at this point in the history
…g. (#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.
  • Loading branch information
huajsj authored and tmoreau89 committed Jun 4, 2019
1 parent 58e15fe commit 38604d9
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 10 deletions.
4 changes: 4 additions & 0 deletions vta/include/vta/driver.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ extern "C" {
#define VTA_MAX_XFER (1<<22)
#endif

/*! PAGE SIZE */
#define VTA_PAGE_BITS 12
#define VTA_PAGE_BYTES (1 << VTA_PAGE_BITS)

/*! \brief Device resource context */
typedef void * VTADeviceHandle;

Expand Down
33 changes: 25 additions & 8 deletions vta/src/runtime.cc
Original file line number Diff line number Diff line change
Expand Up @@ -913,16 +913,33 @@ class CommandQueue {
}

uint32_t GetElemBytes(uint32_t memory_id) {
uint32_t elem_bytes = 0;
switch (memory_id) {
case VTA_MEM_ID_UOP: return VTA_UOP_ELEM_BYTES;
case VTA_MEM_ID_INP: return VTA_INP_ELEM_BYTES;
case VTA_MEM_ID_WGT: return VTA_WGT_ELEM_BYTES;
case VTA_MEM_ID_ACC: return VTA_ACC_ELEM_BYTES;
case VTA_MEM_ID_OUT: return VTA_INP_ELEM_BYTES;
default: break;
case VTA_MEM_ID_UOP:
elem_bytes = VTA_UOP_ELEM_BYTES;
break;
case VTA_MEM_ID_INP:
elem_bytes = VTA_INP_ELEM_BYTES;
break;
case VTA_MEM_ID_WGT:
elem_bytes = VTA_WGT_ELEM_BYTES;
break;
case VTA_MEM_ID_ACC:
elem_bytes = VTA_ACC_ELEM_BYTES;
break;
case VTA_MEM_ID_OUT:
elem_bytes = VTA_INP_ELEM_BYTES;
break;
default:
LOG(FATAL) << "Memory id not recognized:" << memory_id;
break;
}
LOG(FATAL) << "Memory id not recognized:" << memory_id;
return 0;
/*
* elements size should not larger than VTA_PAGE_BYTES.
*
*/
CHECK_GE(VTA_PAGE_BYTES, elem_bytes);
return elem_bytes;
}

void LoadBuffer2D(void* src_dram_addr,
Expand Down
4 changes: 2 additions & 2 deletions vta/src/sim/sim_driver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,9 @@ class DRAM {

private:
// The bits in page table
static constexpr vta_phy_addr_t kPageBits = 12;
static constexpr vta_phy_addr_t kPageBits = VTA_PAGE_BITS;
// page size, also the maximum allocable size 16 K
static constexpr vta_phy_addr_t kPageSize = 1 << kPageBits;
static constexpr vta_phy_addr_t kPageSize = VTA_PAGE_BYTES;
/*! \brief A page in the DRAM */
struct Page {
/*! \brief Data Type */
Expand Down

0 comments on commit 38604d9

Please sign in to comment.