janetsc commented on code in PR #13381:
URL: https://github.com/apache/tvm/pull/13381#discussion_r1022106437
##########
src/runtime/hexagon/hexagon_buffer.cc:
##########
@@ -237,8 +236,15 @@ void hexagon_buffer_copy_across_regions(const BufferSet&
dest, const BufferSet&
// Finally, do the memory copies.
for (const auto& copy : macro_copies) {
- int error_code = hexagon_user_dma_1d_sync(copy.dest, copy.src,
copy.num_bytes);
- CHECK_EQ(error_code, 0);
+ qurt_mem_cache_clean(reinterpret_cast<qurt_addr_t>(copy.dest),
copy.num_bytes,
+ QURT_MEM_CACHE_INVALIDATE, QURT_MEM_DCACHE);
+ qurt_mem_cache_clean(reinterpret_cast<qurt_addr_t>(copy.src),
copy.num_bytes,
+ QURT_MEM_CACHE_INVALIDATE, QURT_MEM_DCACHE);
+ memcpy(copy.dest, copy.src, copy.num_bytes);
+ qurt_mem_cache_clean(reinterpret_cast<qurt_addr_t>(copy.dest),
copy.num_bytes,
Review Comment:
It seems the calls after the copy should not be necessary, particularly for
the src buffer. Are these a temporary measure?
##########
tests/cpp-runtime/hexagon/hexagon_user_dma_tests.cc:
##########
@@ -224,3 +228,124 @@ TEST_F(HexagonUserDMATest, overflow_ring_buffer) {
ASSERT_EQ(src_char[i], dst_char[i]);
}
}
+
+TEST_F(HexagonUserDMATest, sync_dma_bypass) {
+ HexagonBuffer srchb(length, kHexagonAllocAlignment, global_scope);
+ HexagonBuffer dsthb(length, kHexagonAllocAlignment, global_scope);
+ HexagonBuffer vtcmhb(length, kHexagonAllocAlignment, global_vtcm_scope);
+
+ // init src, dst HexagonBuffers
+ srchb.CopyFrom(src, length);
+ dsthb.CopyFrom(dst, length);
+
+ // DDR src -> VTCM
+ ret = user_dma->Copy(queue_id, vtcmhb.GetPointer(), srchb.GetPointer(),
length, ENABLE_BYPASS);
+ ASSERT_EQ(ret, DMA_SUCCESS);
+
+ // VTCM -> DDR dst
+ ret = user_dma->Copy(queue_id, dsthb.GetPointer(), vtcmhb.GetPointer(),
length, ENABLE_BYPASS);
+ ASSERT_EQ(ret, DMA_SUCCESS);
+
+ // wait for DMAs to complete
+ user_dma->Wait(queue_id, 0);
+
+ // copy answer from dst HexagonBuffer
+ dsthb.CopyTo(dst, length);
+
+ // verify
+ for (uint32_t i = 0; i < length; ++i) {
+ ASSERT_EQ(src_char[i], dst_char[i]);
+ }
+}
+
+TEST_F(HexagonUserDMATest, sync_dma_bypass_vtcm_to_vtcm) {
+ HexagonBuffer srchb(length, kHexagonAllocAlignment, global_scope);
+ HexagonBuffer dsthb(length, kHexagonAllocAlignment, global_scope);
+ HexagonBuffer vtcm1hb(length, kHexagonAllocAlignment, global_vtcm_scope);
+ HexagonBuffer vtcm2hb(length, kHexagonAllocAlignment, global_vtcm_scope);
+
+ // init src, dst HexagonBuffers
+ srchb.CopyFrom(src, length);
+ dsthb.CopyFrom(dst, length);
+
+ // DDR src -> VTCM
+ ret = user_dma->Copy(queue_id, vtcm1hb.GetPointer(), srchb.GetPointer(),
length, ENABLE_BYPASS);
+ ASSERT_EQ(ret, DMA_SUCCESS);
+
+ // VTCM -> VTCM
+ // NOTE: Cache bypass is disabled for VTCM -> VTCM transfers
+ ret =
+ user_dma->Copy(queue_id, vtcm2hb.GetPointer(), vtcm1hb.GetPointer(),
length, DISABLE_BYPASS);
Review Comment:
Since this is the enabled test, should it be ENABLE_BYPASS?
Edit - Ah, I see the comment. But you should be able to say ENABLED, and
have it still work. It just won't actually enable anything. Or, should it
through if you attempt to set bypass if it is vtcm to vtcm?
##########
src/runtime/hexagon/hexagon_vtcm_pool.h:
##########
@@ -70,6 +70,16 @@ class HexagonVtcmPool {
//! \brief Returns the total number of bytes in this pool
size_t TotalBytes() { return reinterpret_cast<size_t>(vtcm_size_); }
+ bool IsVtcm(void* ptr, unsigned size) {
+ auto char_ptr = static_cast<char*>(ptr);
+ auto char_vtcm = static_cast<char*>(vtcm_data_);
+
Review Comment:
Add CHECK(vtcm_data_ != nullptr)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
