[Bugfix] [VTA] VTA DRAM Have A Logic Issue May Cause GEMM Output Wrong. (#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.

diff --git a/vta/include/vta/driver.h b/vta/include/vta/driver.h
index ed04185..d583051 100644 (file)
--- a/vta/include/vta/driver.h
+++ b/vta/include/vta/driver.h
@@ -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;
 

diff --git a/vta/src/runtime.cc b/vta/src/runtime.cc
index 7af0de1..79a407f 100644 (file)
--- a/vta/src/runtime.cc
+++ b/vta/src/runtime.cc
@@ -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,

diff --git a/vta/src/sim/sim_driver.cc b/vta/src/sim/sim_driver.cc
index 803a54d..5f9f6b6 100644 (file)
--- a/vta/src/sim/sim_driver.cc
+++ b/vta/src/sim/sim_driver.cc
@@ -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 */