You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/10/06 15:46:24 UTC

[GitHub] [tvm] janetsc opened a new pull request, #12999: [Hexagon] [runtime] Support VTCM alignments of 128 or 2k

janetsc opened a new pull request, #12999:
URL: https://github.com/apache/tvm/pull/12999

   In order to simplify the dynamic VTCM memory pool, it will support the following alignments:
   
   2k boundaries, allocated from the front of the pool in the smallest open spot possible.
   128 byte boundaries, allocated from the end of the pool.
   
   Made this support explicit, modified test to hit these boundaries.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] janetsc commented on a diff in pull request #12999: [Hexagon] [runtime] Support VTCM alignments of 128 or 2k

Posted by GitBox <gi...@apache.org>.
janetsc commented on code in PR #12999:
URL: https://github.com/apache/tvm/pull/12999#discussion_r992579233


##########
tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc:
##########
@@ -25,50 +25,66 @@ using namespace tvm::runtime;
 using namespace tvm::runtime::hexagon;
 
 class HexagonVtcmPoolTest : public ::testing::Test {

Review Comment:
   Yep!  I added some in TEST_F(HexagonVtcmPoolTest, vtcm_alignment) with my latest commits.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] kparzysz-quic commented on a diff in pull request #12999: [Hexagon] [runtime] Support VTCM alignments of 128 or 2k

Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on code in PR #12999:
URL: https://github.com/apache/tvm/pull/12999#discussion_r992368913


##########
src/runtime/hexagon/hexagon_buffer.cc:
##########
@@ -57,17 +57,30 @@ struct DDRAllocation : public Allocation {
 
 struct VTCMAllocation : public Allocation {
   VTCMAllocation(size_t nbytes, size_t alignment) : Allocation(nbytes, alignment) {
-    // TODO(HWE): Handle alignments greater than 2k
-    CHECK(alignment <= 0x800) << "VTCMAllocation called for invalid alignment";
-    if ((nbytes & 0x7FF) && ((alignment & 0x7FF) == 0)) {
-      // Caller has requested 2k alignment, but the size is not a multiple of 2k
-      // Adjust size to be a multiple of 2k so that we will allocate from the front of the pool
+    // For simplicity, the current VTCM dynamic pool supports the following alignments: less than
+    // or equal to 128 (0x80), and 2k (0x800)
+    CHECK((alignment <= 0x80) || (alignment == 0x800))
+        << "VTCMAllocation called for invalid alignment " << alignment;
+
+    if ((alignment == 0x800) && (nbytes & 0x7FF)) {
+      // Caller has requested 2k alignment, but the size is not a multiple of 2k.
+      // Adjust size to be a multiple of 2k so that we will allocate from the front of the pool.
       nbytes = nbytes >> 11;
       nbytes = nbytes << 11;
       nbytes += 0x800;

Review Comment:
   Side note---an alternative would be `nbytes = (nbytes + 0x7ff) & -0x800`, and it would work without the check if `nbytes & 0x7ff`.  Same below.



##########
tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc:
##########
@@ -25,50 +25,66 @@ using namespace tvm::runtime;
 using namespace tvm::runtime::hexagon;
 
 class HexagonVtcmPoolTest : public ::testing::Test {
-  void SetUp() override { vtcm_pool = HexagonDeviceAPI::Global()->VtcmPool(); }
+  void SetUp() override {
+    vtcm_pool = HexagonDeviceAPI::Global()->VtcmPool();
+    max_bytes = vtcm_pool->TotalBytes();
+  }
   void TearDown() override {}
 
  public:
   HexagonVtcmPool* vtcm_pool;
+  size_t max_bytes;
+  size_t two_k_block = 2048;
+  size_t one_k_block = 1024;
+  size_t min_bytes = 128;
 };
 
 TEST_F(HexagonVtcmPoolTest, basic) {
   void* ptr;
-  size_t max_bytes = vtcm_pool->TotalBytes();
-  size_t two_k_block = 2048;
-  size_t one_k_block = 1024;
-  size_t one_byte_block = 1;
+  void* ptr2;
+
   ptr = vtcm_pool->Allocate(max_bytes);
+  CHECK((reinterpret_cast<size_t>(ptr) & 0x7FF) == 0)

Review Comment:
   Please use `uintptr_t` (or `intptr_t`) for integer representations of pointers (these types are exactly for that).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] driazati merged pull request #12999: [Hexagon] [runtime] Support VTCM alignments of 128 or 2k

Posted by GitBox <gi...@apache.org>.
driazati merged PR #12999:
URL: https://github.com/apache/tvm/pull/12999


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] nverke commented on a diff in pull request #12999: [Hexagon] [runtime] Support VTCM alignments of 128 or 2k

Posted by GitBox <gi...@apache.org>.
nverke commented on code in PR #12999:
URL: https://github.com/apache/tvm/pull/12999#discussion_r992514453


##########
tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc:
##########
@@ -25,50 +25,66 @@ using namespace tvm::runtime;
 using namespace tvm::runtime::hexagon;
 
 class HexagonVtcmPoolTest : public ::testing::Test {

Review Comment:
   I dont see any tests covering allocations with odd alignments. Could we add some that would need to have their alignment adjusted? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] janetsc commented on pull request #12999: [Hexagon] [runtime] Support VTCM alignments of 128 or 2k

Posted by GitBox <gi...@apache.org>.
janetsc commented on PR #12999:
URL: https://github.com/apache/tvm/pull/12999#issuecomment-1275102656

   @tvm-bot rerun


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org