Skip to content

[Offload] Have olMemFree accept a platform as a param #157478

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

[Offload] Have olMemFree accept a platform as a param #157478

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0dc7345
[Offload] Have olMemFree accept a platform as a param
RossBrunton Sep 8, 2025
99b0261
[Offload][Conformance] Update olMemFree calls in conformance tests (#…
leandrolcampos Sep 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion offload/liboffload/API/Memory.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,12 @@ def olMemAlloc : Function {
def olMemFree : Function {
let desc = "Frees a memory allocation previously made by olMemAlloc.";
let params = [
Param<"ol_platform_handle_t", "Platform", "handle of the platform that allocated this memory", PARAM_IN>,
Param<"void*", "Address", "address of the allocation to free", PARAM_IN>,
];
let returns = [];
let returns = [
Return<"OL_ERRC_NOT_FOUND", ["memory was not allocated by this platform"]>
];
}

def olMemcpy : Function {
Expand Down
3 changes: 2 additions & 1 deletion offload/liboffload/src/OffloadImpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ Error olMemAlloc_impl(ol_device_handle_t Device, ol_alloc_type_t Type,
return Error::success();
}

Error olMemFree_impl(void *Address) {
Error olMemFree_impl(ol_platform_handle_t Platform, void *Address) {
ol_device_handle_t Device;
ol_alloc_type_t Type;
{
Expand All @@ -646,6 +646,7 @@ Error olMemFree_impl(void *Address) {
Type = AllocInfo.Type;
OffloadContext::get().AllocInfoMap.erase(Address);
}
assert(Platform == Device->Platform);

if (auto Res =
Device->Device->dataDelete(Address, convertOlToPluginAllocTy(Type)))
Expand Down
8 changes: 6 additions & 2 deletions offload/unittests/Conformance/include/mathtest/DeviceContext.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,13 @@ class DeviceContext {
explicit DeviceContext(llvm::StringRef Platform, std::size_t DeviceId = 0);

template <typename T>
ManagedBuffer<T> createManagedBuffer(std::size_t Size) const noexcept {
ManagedBuffer<T> createManagedBuffer(std::size_t Size) noexcept {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leandrolcampos I've needed to touch a lot of the conformance tests here, but for some reason I'm unable to coerce it into being built and tested.

Can you have a look and let me know if all of this looks okay?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @RossBrunton,
I'll take a look until the end of the day.

Copy link
Contributor

@leandrolcampos leandrolcampos Sep 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RossBrunton, could you take a look at this #157773? Merging this patch will ensure the conformance tests run correctly.

I also prepared a Gist containing a step-by-step guide to setting up an environment to build and run the conformance tests on Windows Subsystem for Linux (WSL 2) with NVIDIA GPUs. I think it can be easily adapted to different OS and/or platform configurations.

Finally, thanks for your care with the conformance tests! If you're interested in the broader context of this work, I recently wrote about it in this post on the LLVM Blog.

void *UntypedAddress = nullptr;

detail::allocManagedMemory(DeviceHandle, Size * sizeof(T), &UntypedAddress);
T *TypedAddress = static_cast<T *>(UntypedAddress);

return ManagedBuffer<T>(TypedAddress, Size);
return ManagedBuffer<T>(getPlatformHandle(), TypedAddress, Size);
}

[[nodiscard]] llvm::Expected<std::shared_ptr<DeviceImage>>
Expand Down Expand Up @@ -120,6 +120,9 @@ class DeviceContext {

[[nodiscard]] llvm::StringRef getPlatform() const noexcept;

[[nodiscard]] llvm::Expected<ol_platform_handle_t>
getPlatformHandle() noexcept;

private:
[[nodiscard]] llvm::Expected<ol_symbol_handle_t>
getKernelHandle(ol_program_handle_t ProgramHandle,
Expand All @@ -131,6 +134,7 @@ class DeviceContext {

std::size_t GlobalDeviceId;
ol_device_handle_t DeviceHandle;
ol_platform_handle_t PlatformHandle = nullptr;
};
} // namespace mathtest

Expand Down
12 changes: 7 additions & 5 deletions offload/unittests/Conformance/include/mathtest/DeviceResources.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class DeviceContext;

namespace detail {

void freeDeviceMemory(void *Address) noexcept;
void freeDeviceMemory(ol_platform_handle_t Platform, void *Address) noexcept;
} // namespace detail

//===----------------------------------------------------------------------===//
Expand All @@ -40,7 +40,7 @@ template <typename T> class [[nodiscard]] ManagedBuffer {
public:
~ManagedBuffer() noexcept {
if (Address)
detail::freeDeviceMemory(Address);
detail::freeDeviceMemory(Platform, Address);
}

ManagedBuffer(const ManagedBuffer &) = delete;
Expand All @@ -57,7 +57,7 @@ template <typename T> class [[nodiscard]] ManagedBuffer {
return *this;

if (Address)
detail::freeDeviceMemory(Address);
detail::freeDeviceMemory(Platform, Address);

Address = Other.Address;
Size = Other.Size;
Expand Down Expand Up @@ -85,9 +85,11 @@ template <typename T> class [[nodiscard]] ManagedBuffer {
private:
friend class DeviceContext;

explicit ManagedBuffer(T *Address, std::size_t Size) noexcept
: Address(Address), Size(Size) {}
explicit ManagedBuffer(ol_platform_handle_t Platform, T *Address,
std::size_t Size) noexcept
: Platform(Platform), Address(Address), Size(Size) {}

ol_platform_handle_t Platform;
T *Address = nullptr;
std::size_t Size = 0;
};
Expand Down
4 changes: 2 additions & 2 deletions offload/unittests/Conformance/include/mathtest/GpuMathTest.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class [[nodiscard]] GpuMathTest final {

ResultType run(GeneratorType &Generator,
std::size_t BufferSize = DefaultBufferSize,
uint32_t GroupSize = DefaultGroupSize) const noexcept {
uint32_t GroupSize = DefaultGroupSize) noexcept {
assert(BufferSize > 0 && "Buffer size must be a positive value");
assert(GroupSize > 0 && "Group size must be a positive value");

Expand Down Expand Up @@ -128,7 +128,7 @@ class [[nodiscard]] GpuMathTest final {
return *ExpectedKernel;
}

[[nodiscard]] auto createBuffers(std::size_t BufferSize) const {
[[nodiscard]] auto createBuffers(std::size_t BufferSize) {
auto InBuffersTuple = std::apply(
[&](auto... InTypeIdentities) {
return std::make_tuple(
Expand Down
3 changes: 3 additions & 0 deletions offload/unittests/Conformance/include/mathtest/OffloadForward.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ typedef struct ol_program_impl_t *ol_program_handle_t;
struct ol_symbol_impl_t;
typedef struct ol_symbol_impl_t *ol_symbol_handle_t;

struct ol_platform_impl_t;
typedef struct ol_platform_impl_t *ol_platform_handle_t;

#ifdef __cplusplus
}
#endif // __cplusplus
Expand Down
23 changes: 23 additions & 0 deletions offload/unittests/Conformance/lib/DeviceContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,29 @@ DeviceContext::getKernelHandle(ol_program_handle_t ProgramHandle,
return Handle;
}

llvm::Expected<ol_platform_handle_t>
DeviceContext::getPlatformHandle() noexcept {
if (!PlatformHandle) {
const ol_result_t OlResult =
olGetDeviceInfo(DeviceHandle, OL_DEVICE_INFO_PLATFORM,
sizeof(PlatformHandle), &PlatformHandle);

if (OlResult != OL_SUCCESS) {
PlatformHandle = nullptr;
llvm::StringRef Details =
OlResult->Details ? OlResult->Details : "No details provided";

// clang-format off
return llvm::createStringError(
llvm::Twine(Details) +
" (code " + llvm::Twine(OlResult->Code) + ")");
// clang-format on
}
}

return PlatformHandle;
}

void DeviceContext::launchKernelImpl(
ol_symbol_handle_t KernelHandle, uint32_t NumGroups, uint32_t GroupSize,
const void *KernelArgs, std::size_t KernelArgsSize) const noexcept {
Expand Down
5 changes: 3 additions & 2 deletions offload/unittests/Conformance/lib/DeviceResources.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@ using namespace mathtest;
// Helpers
//===----------------------------------------------------------------------===//

void detail::freeDeviceMemory(void *Address) noexcept {
void detail::freeDeviceMemory(ol_platform_handle_t Platform,
void *Address) noexcept {
if (Address)
OL_CHECK(olMemFree(Address));
OL_CHECK(olMemFree(Platform, Address));
}

//===----------------------------------------------------------------------===//
Expand Down
8 changes: 4 additions & 4 deletions offload/unittests/OffloadAPI/common/Fixtures.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,20 +137,20 @@ struct OffloadDeviceTest
Device = DeviceParam.Handle;
if (Device == nullptr)
GTEST_SKIP() << "No available devices.";

ASSERT_SUCCESS(olGetDeviceInfo(Device, OL_DEVICE_INFO_PLATFORM,
sizeof(ol_platform_handle_t), &Platform));
}

ol_platform_backend_t getPlatformBackend() const {
ol_platform_handle_t Platform = nullptr;
if (olGetDeviceInfo(Device, OL_DEVICE_INFO_PLATFORM,
sizeof(ol_platform_handle_t), &Platform))
return OL_PLATFORM_BACKEND_UNKNOWN;
ol_platform_backend_t Backend;
if (olGetPlatformInfo(Platform, OL_PLATFORM_INFO_BACKEND,
sizeof(ol_platform_backend_t), &Backend))
return OL_PLATFORM_BACKEND_UNKNOWN;
return Backend;
}

ol_platform_handle_t Platform = nullptr;
ol_device_handle_t Device = nullptr;
};

Expand Down
16 changes: 8 additions & 8 deletions offload/unittests/OffloadAPI/kernel/olLaunchKernel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ TEST_P(olLaunchKernelFooTest, Success) {
ASSERT_EQ(Data[i], i);
}

ASSERT_SUCCESS(olMemFree(Mem));
ASSERT_SUCCESS(olMemFree(Platform, Mem));
}

TEST_P(olLaunchKernelFooTest, SuccessThreaded) {
Expand All @@ -123,7 +123,7 @@ TEST_P(olLaunchKernelFooTest, SuccessThreaded) {
ASSERT_EQ(Data[i], i);
}

ASSERT_SUCCESS(olMemFree(Mem));
ASSERT_SUCCESS(olMemFree(Platform, Mem));
});
}

Expand Down Expand Up @@ -151,7 +151,7 @@ TEST_P(olLaunchKernelFooTest, SuccessSynchronous) {
ASSERT_EQ(Data[i], i);
}

ASSERT_SUCCESS(olMemFree(Mem));
ASSERT_SUCCESS(olMemFree(Platform, Mem));
}

TEST_P(olLaunchKernelLocalMemTest, Success) {
Expand All @@ -176,7 +176,7 @@ TEST_P(olLaunchKernelLocalMemTest, Success) {
for (uint32_t i = 0; i < LaunchArgs.GroupSize.x * LaunchArgs.NumGroups.x; i++)
ASSERT_EQ(Data[i], (i % 64) * 2);

ASSERT_SUCCESS(olMemFree(Mem));
ASSERT_SUCCESS(olMemFree(Platform, Mem));
}

TEST_P(olLaunchKernelLocalMemReductionTest, Success) {
Expand All @@ -199,7 +199,7 @@ TEST_P(olLaunchKernelLocalMemReductionTest, Success) {
for (uint32_t i = 0; i < LaunchArgs.NumGroups.x; i++)
ASSERT_EQ(Data[i], 2 * LaunchArgs.GroupSize.x);

ASSERT_SUCCESS(olMemFree(Mem));
ASSERT_SUCCESS(olMemFree(Platform, Mem));
}

TEST_P(olLaunchKernelLocalMemStaticTest, Success) {
Expand All @@ -222,7 +222,7 @@ TEST_P(olLaunchKernelLocalMemStaticTest, Success) {
for (uint32_t i = 0; i < LaunchArgs.NumGroups.x; i++)
ASSERT_EQ(Data[i], 2 * LaunchArgs.GroupSize.x);

ASSERT_SUCCESS(olMemFree(Mem));
ASSERT_SUCCESS(olMemFree(Platform, Mem));
}

TEST_P(olLaunchKernelGlobalTest, Success) {
Expand All @@ -245,7 +245,7 @@ TEST_P(olLaunchKernelGlobalTest, Success) {
ASSERT_EQ(Data[i], i * 2);
}

ASSERT_SUCCESS(olMemFree(Mem));
ASSERT_SUCCESS(olMemFree(Platform, Mem));
}

TEST_P(olLaunchKernelGlobalTest, InvalidNotAKernel) {
Expand Down Expand Up @@ -273,7 +273,7 @@ TEST_P(olLaunchKernelGlobalCtorTest, Success) {
ASSERT_EQ(Data[i], i + 100);
}

ASSERT_SUCCESS(olMemFree(Mem));
ASSERT_SUCCESS(olMemFree(Platform, Mem));
}

TEST_P(olLaunchKernelGlobalDtorTest, Success) {
Expand Down
6 changes: 3 additions & 3 deletions offload/unittests/OffloadAPI/memory/olMemAlloc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,21 @@ TEST_P(olMemAllocTest, SuccessAllocManaged) {
void *Alloc = nullptr;
ASSERT_SUCCESS(olMemAlloc(Device, OL_ALLOC_TYPE_MANAGED, 1024, &Alloc));
ASSERT_NE(Alloc, nullptr);
olMemFree(Alloc);
olMemFree(Platform, Alloc);
}

TEST_P(olMemAllocTest, SuccessAllocHost) {
void *Alloc = nullptr;
ASSERT_SUCCESS(olMemAlloc(Device, OL_ALLOC_TYPE_HOST, 1024, &Alloc));
ASSERT_NE(Alloc, nullptr);
olMemFree(Alloc);
olMemFree(Platform, Alloc);
}

TEST_P(olMemAllocTest, SuccessAllocDevice) {
void *Alloc = nullptr;
ASSERT_SUCCESS(olMemAlloc(Device, OL_ALLOC_TYPE_DEVICE, 1024, &Alloc));
ASSERT_NE(Alloc, nullptr);
olMemFree(Alloc);
olMemFree(Platform, Alloc);
}

TEST_P(olMemAllocTest, InvalidNullDevice) {
Expand Down
12 changes: 6 additions & 6 deletions offload/unittests/OffloadAPI/memory/olMemFill.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ struct olMemFillTest : OffloadQueueTest {
ASSERT_EQ(AllocPtr[i], Pattern);
}

olMemFree(Alloc);
olMemFree(Platform, Alloc);
}
};
OFFLOAD_TESTS_INSTANTIATE_DEVICE_FIXTURE(olMemFillTest);
Expand Down Expand Up @@ -92,7 +92,7 @@ TEST_P(olMemFillTest, SuccessLarge) {
ASSERT_EQ(AllocPtr[i].B, UINT64_MAX);
}

olMemFree(Alloc);
olMemFree(Platform, Alloc);
}

TEST_P(olMemFillTest, SuccessLargeEnqueue) {
Expand Down Expand Up @@ -120,7 +120,7 @@ TEST_P(olMemFillTest, SuccessLargeEnqueue) {
ASSERT_EQ(AllocPtr[i].B, UINT64_MAX);
}

olMemFree(Alloc);
olMemFree(Platform, Alloc);
}

TEST_P(olMemFillTest, SuccessLargeByteAligned) {
Expand All @@ -146,7 +146,7 @@ TEST_P(olMemFillTest, SuccessLargeByteAligned) {
ASSERT_EQ(AllocPtr[i].C, 255);
}

olMemFree(Alloc);
olMemFree(Platform, Alloc);
}

TEST_P(olMemFillTest, SuccessLargeByteAlignedEnqueue) {
Expand Down Expand Up @@ -176,7 +176,7 @@ TEST_P(olMemFillTest, SuccessLargeByteAlignedEnqueue) {
ASSERT_EQ(AllocPtr[i].C, 255);
}

olMemFree(Alloc);
olMemFree(Platform, Alloc);
}

TEST_P(olMemFillTest, InvalidPatternSize) {
Expand All @@ -189,5 +189,5 @@ TEST_P(olMemFillTest, InvalidPatternSize) {
olMemFill(Queue, Alloc, sizeof(Pattern), &Pattern, Size));

olSyncQueue(Queue);
olMemFree(Alloc);
olMemFree(Platform, Alloc);
}
17 changes: 12 additions & 5 deletions offload/unittests/OffloadAPI/memory/olMemFree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,31 @@ OFFLOAD_TESTS_INSTANTIATE_DEVICE_FIXTURE(olMemFreeTest);
TEST_P(olMemFreeTest, SuccessFreeManaged) {
void *Alloc = nullptr;
ASSERT_SUCCESS(olMemAlloc(Device, OL_ALLOC_TYPE_MANAGED, 1024, &Alloc));
ASSERT_SUCCESS(olMemFree(Alloc));
ASSERT_SUCCESS(olMemFree(Platform, Alloc));
}

TEST_P(olMemFreeTest, SuccessFreeHost) {
void *Alloc = nullptr;
ASSERT_SUCCESS(olMemAlloc(Device, OL_ALLOC_TYPE_HOST, 1024, &Alloc));
ASSERT_SUCCESS(olMemFree(Alloc));
ASSERT_SUCCESS(olMemFree(Platform, Alloc));
}

TEST_P(olMemFreeTest, SuccessFreeDevice) {
void *Alloc = nullptr;
ASSERT_SUCCESS(olMemAlloc(Device, OL_ALLOC_TYPE_DEVICE, 1024, &Alloc));
ASSERT_SUCCESS(olMemFree(Alloc));
ASSERT_SUCCESS(olMemFree(Platform, Alloc));
}

TEST_P(olMemFreeTest, InvalidNullPtr) {
void *Alloc = nullptr;
ASSERT_SUCCESS(olMemAlloc(Device, OL_ALLOC_TYPE_DEVICE, 1024, &Alloc));
ASSERT_ERROR(OL_ERRC_INVALID_NULL_POINTER, olMemFree(nullptr));
ASSERT_SUCCESS(olMemFree(Alloc));
ASSERT_ERROR(OL_ERRC_INVALID_NULL_POINTER, olMemFree(Platform, nullptr));
ASSERT_SUCCESS(olMemFree(Platform, Alloc));
}

TEST_P(olMemFreeTest, InvalidPlatformPtr) {
void *Alloc = nullptr;
ASSERT_SUCCESS(olMemAlloc(Device, OL_ALLOC_TYPE_DEVICE, 1024, &Alloc));
ASSERT_ERROR(OL_ERRC_INVALID_NULL_HANDLE, olMemFree(nullptr, Alloc));
ASSERT_SUCCESS(olMemFree(Platform, Alloc));
}
Loading
Loading