adstraw commented on code in PR #13844:
URL: https://github.com/apache/tvm/pull/13844#discussion_r1091229524
##########
src/runtime/hexagon/hexagon_buffer.cc:
##########
@@ -49,6 +49,16 @@ struct Allocation {
struct DDRAllocation : public Allocation {
DDRAllocation(size_t nbytes, size_t alignment) : Allocation(nbytes,
alignment) {
int ret = posix_memalign(&data_, alignment, nbytes);
+
+ // The heap used by malloc on Hexagon is always mapped as cacheable. The
heap manager may
+ // not perform cache flush and invalidation on a prior memory free. So, a
subsequent memory
+ // allocation request to the heap manager may allocate memory that resides
in part or in full in
+ // the cache. Hence, we must flush and invalidate the allocation from the
cache to ensure that
+ // DMA with cache bypass enabled will function properly. DMA with cache
bypass enabled assumes
+ // that HexagonBuffer objects are not cached unless explicitly modified by
the primfunc. We must
+ // flush and invalidate after malloc to uphold this assumption.
+ qurt_mem_cache_clean(reinterpret_cast<qurt_addr_t>(data_), nbytes,
+ QURT_MEM_CACHE_FLUSH_INVALIDATE, QURT_MEM_DCACHE);
Review Comment:
Without going into too much detail, I believe that invalidate (only) maps to
"flush and invalidate" when data is modified in the cache. Before this PR we
were doing invalidate (only) on the destination of the `memcpy` which is
modified in cache which should not have worked unless the modified data was
first flushed from the cache. Still, using "invalidate" for unmodified data
and "flush and invalidate" for modified data is semantically correct and makes
the code more readable so I will follow that convention.
--
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]
