Skip to content

Commit

Permalink
Feedback from Morten
Browse files Browse the repository at this point in the history
  • Loading branch information
teqdruid committed Jan 24, 2025
1 parent ef7e3c0 commit 92174df
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 7 deletions.
6 changes: 4 additions & 2 deletions lib/Dialect/ESI/runtime/cpp/include/esi/Services.h
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,10 @@ class HostMem : public Service {
operator void *() const { return getPtr(); }
virtual std::size_t getSize() const = 0;
/// Flush the memory region to ensure that the device sees the latest
/// contents. Recommended before DMA transactions, though not all platforms
/// require it.
/// contents. Because some platforms require it before DMA transactions, it
/// is recommended to call this before any DMA on all platforms. On
/// platforms which don't require it, it is a cheap no-op virtual method
/// call.
virtual void flush() {}
};

Expand Down
13 changes: 9 additions & 4 deletions lib/Dialect/ESI/runtime/cpp/lib/backends/Xrt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,16 @@ struct esi::backends::xrt::XrtAccelerator::Impl {
// Find memory group for the host.
::xrt::xclbin xcl(xclbin);
std::optional<::xrt::xclbin::mem> host_mem;
for (auto mem : xcl.get_mems())
for (auto mem : xcl.get_mems()) {
// The host memory is tagged with "HOST[0]". Memory type is wrong --
// reports as DRAM rather than host memory so we can't filter on that.
if (mem.get_tag().starts_with("HOST"))
host_mem = mem;
if (mem.get_tag().starts_with("HOST")) {
if (host_mem.has_value())
throw std::runtime_error("Multiple host memories found in xclbin");
else
host_mem = mem;
}
}
if (!host_mem)
throw std::runtime_error("No host memory found in xclbin");
memoryGroup = host_mem->get_index();
Expand Down Expand Up @@ -135,7 +140,7 @@ class XrtHostMem : public HostMem {
/// the pointer the user application sees.
virtual void *getDevicePtr() const override { return (void *)bo.address(); }
virtual std::size_t getSize() const override { return bo.size(); }
/// It is recommended to use 'sync' to flush the caches before executing any
/// It is required to use 'sync' to flush the caches before executing any
/// DMA.
virtual void flush() override { bo.sync(XCL_BO_SYNC_BO_TO_DEVICE); }

Expand Down
2 changes: 1 addition & 1 deletion lib/Dialect/ESI/runtime/cpp/tools/esitester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ void dmaTest(Accelerator *acc, esi::services::HostMem::HostMemRegion &region,
dataPtr[0] = 0x12345678 << i;
dataPtr[1] = 0xDEADBEEF << i;
region.flush();
readMem->write(8, (uint64_t)devicePtr);
readMem->write(8, reinterpret_cast<uint64_t>(devicePtr));

// Wait for the accelerator to read the correct value. Timeout and fail
// after 10ms.
Expand Down

0 comments on commit 92174df

Please sign in to comment.