Skip to content

Commit

Permalink
[StreamExecutor] Fix allocateDeviceMemory
Browse files Browse the repository at this point in the history
Summary:
The return value from PlatformExecutor::allocateDeviceMemory needs to be
converted from Expected<GlobalDeviceMemoryBase> to
Expected<GlobalDeviceMemory<T>> in Executor::allocateDeviceMemory.

A similar bug is also fixed for Executor::allocateHostMemory.

Thanks to jprice for identifying this bug.

Reviewers: jprice, jlebar

Subscribers: parallel_libs-commits

Differential Revision: https://reviews.llvm.org/D23849

llvm-svn: 279658
  • Loading branch information
henline committed Aug 24, 2016
1 parent 4a080de commit 3053bbf
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 2 deletions.
12 changes: 10 additions & 2 deletions parallel-libs/streamexecutor/include/streamexecutor/Executor.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ class Executor {
/// Allocates an array of ElementCount entries of type T in device memory.
template <typename T>
Expected<GlobalDeviceMemory<T>> allocateDeviceMemory(size_t ElementCount) {
return PExecutor->allocateDeviceMemory(ElementCount * sizeof(T));
Expected<GlobalDeviceMemoryBase> MaybeBase =
PExecutor->allocateDeviceMemory(ElementCount * sizeof(T));
if (!MaybeBase)
return MaybeBase.takeError();
return GlobalDeviceMemory<T>(*MaybeBase);
}

/// Frees memory previously allocated with allocateDeviceMemory.
Expand All @@ -54,7 +58,11 @@ class Executor {
/// Host memory allocated by this function can be used for asynchronous memory
/// copies on streams. See Stream::thenCopyD2H and Stream::thenCopyH2D.
template <typename T> Expected<T *> allocateHostMemory(size_t ElementCount) {
return PExecutor->allocateHostMemory(ElementCount * sizeof(T));
Expected<void *> MaybeMemory =
PExecutor->allocateHostMemory(ElementCount * sizeof(T));
if (!MaybeMemory)
return MaybeMemory.takeError();
return static_cast<T *>(*MaybeMemory);
}

/// Frees memory previously allocated with allocateHostMemory.
Expand Down
27 changes: 27 additions & 0 deletions parallel-libs/streamexecutor/lib/unittests/ExecutorTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,14 @@ class MockPlatformExecutor : public se::PlatformExecutor {
return se::Error::success();
}

se::Error registerHostMemory(void *, size_t) override {
return se::Error::success();
}

se::Error unregisterHostMemory(void *) override {
return se::Error::success();
}

se::Error synchronousCopyD2H(const se::GlobalDeviceMemoryBase &DeviceSrc,
size_t SrcByteOffset, void *HostDst,
size_t DstByteOffset,
Expand Down Expand Up @@ -131,6 +139,25 @@ class ExecutorTest : public ::testing::Test {
using llvm::ArrayRef;
using llvm::MutableArrayRef;

TEST_F(ExecutorTest, AllocateAndFreeDeviceMemory) {
se::Expected<se::GlobalDeviceMemory<int>> MaybeMemory =
Executor.allocateDeviceMemory<int>(10);
EXPECT_TRUE(static_cast<bool>(MaybeMemory));
EXPECT_NO_ERROR(Executor.freeDeviceMemory(*MaybeMemory));
}

TEST_F(ExecutorTest, AllocateAndFreeHostMemory) {
se::Expected<int *> MaybeMemory = Executor.allocateHostMemory<int>(10);
EXPECT_TRUE(static_cast<bool>(MaybeMemory));
EXPECT_NO_ERROR(Executor.freeHostMemory(*MaybeMemory));
}

TEST_F(ExecutorTest, RegisterAndUnregisterHostMemory) {
std::vector<int> Data(10);
EXPECT_NO_ERROR(Executor.registerHostMemory(Data.data(), 10));
EXPECT_NO_ERROR(Executor.unregisterHostMemory(Data.data()));
}

// D2H tests

TEST_F(ExecutorTest, SyncCopyD2HToMutableArrayRefByCount) {
Expand Down

0 comments on commit 3053bbf

Please sign in to comment.