-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[libc] Efficiently implement 'realloc' for AMDGPU devices #145960
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ | |
#include "src/__support/GPU/utils.h" | ||
#include "src/__support/RPC/rpc_client.h" | ||
#include "src/__support/threads/sleep.h" | ||
#include "src/string/memory_utils/inline_memcpy.h" | ||
|
||
namespace LIBC_NAMESPACE_DECL { | ||
|
||
|
@@ -550,5 +551,26 @@ void deallocate(void *ptr) { | |
release_slab(slab); | ||
} | ||
|
||
void *reallocate(void *ptr, uint64_t size) { | ||
if (ptr == nullptr) | ||
return gpu::allocate(size); | ||
|
||
// Non-slab allocations are considered foreign pointers so we fail. | ||
if ((reinterpret_cast<uintptr_t>(ptr) & SLAB_ALIGNMENT) == 0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should use __builtin_is_aligned instead of introducing ptrtoint for alignment checks There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, can I do that in a follow-up? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure, I assume there are other places that should update too |
||
return nullptr; | ||
|
||
// The original slab pointer is the 2MiB boundary using the given pointer. | ||
Slab *slab = cpp::launder(reinterpret_cast<Slab *>( | ||
(reinterpret_cast<uintptr_t>(ptr) & ~SLAB_ALIGNMENT))); | ||
if (slab->get_chunk_size() >= size) | ||
return ptr; | ||
|
||
// If we need a new chunk we reallocate and copy it over. | ||
void *new_ptr = gpu::allocate(size); | ||
inline_memcpy(new_ptr, ptr, slab->get_chunk_size()); | ||
gpu::deallocate(ptr); | ||
return new_ptr; | ||
} | ||
|
||
} // namespace gpu | ||
} // namespace LIBC_NAMESPACE_DECL |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
#include "test/IntegrationTest/test.h" | ||
|
||
#include "src/__support/GPU/utils.h" | ||
#include "src/stdlib/free.h" | ||
#include "src/stdlib/malloc.h" | ||
#include "src/stdlib/realloc.h" | ||
|
||
using namespace LIBC_NAMESPACE; | ||
|
||
TEST_MAIN(int, char **, char **) { | ||
// realloc(nullptr, size) is equivalent to malloc. | ||
int *alloc = reinterpret_cast<int *>(LIBC_NAMESPACE::realloc(nullptr, 32)); | ||
EXPECT_NE(alloc, nullptr); | ||
*alloc = 42; | ||
EXPECT_EQ(*alloc, 42); | ||
|
||
// realloc to same size returns the same pointer. | ||
void *same = LIBC_NAMESPACE::realloc(alloc, 32); | ||
EXPECT_EQ(same, alloc); | ||
EXPECT_EQ(reinterpret_cast<int *>(same)[0], 42); | ||
|
||
// realloc to smaller size returns same pointer. | ||
void *smaller = LIBC_NAMESPACE::realloc(same, 16); | ||
EXPECT_EQ(smaller, alloc); | ||
EXPECT_EQ(reinterpret_cast<int *>(smaller)[0], 42); | ||
|
||
// realloc to larger size returns new pointer and preserves contents. | ||
int *larger = reinterpret_cast<int *>(LIBC_NAMESPACE::realloc(smaller, 128)); | ||
EXPECT_NE(larger, nullptr); | ||
EXPECT_EQ(larger[0], 42); | ||
|
||
// realloc works when called with a divergent size. | ||
int *div = reinterpret_cast<int *>( | ||
LIBC_NAMESPACE::malloc((gpu::get_thread_id() + 1) * 16)); | ||
EXPECT_NE(div, nullptr); | ||
div[0] = static_cast<int>(gpu::get_thread_id()); | ||
int *div_realloc = reinterpret_cast<int *>( | ||
LIBC_NAMESPACE::realloc(div, ((gpu::get_thread_id() + 1) * 32))); | ||
EXPECT_NE(div_realloc, nullptr); | ||
EXPECT_EQ(div_realloc[0], static_cast<int>(gpu::get_thread_id())); | ||
LIBC_NAMESPACE::free(div_realloc); | ||
|
||
return 0; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be size_t?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's
size_t
in the interface to make the standard happy, but we just cast it to 64-bit immediately since that's what it is for the targets we support.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see how that's a plus, just use size_t everywhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It made it clearer to me because I intentionally truncate it to
uint32_t
internally once we enter the chunk-based allocator to save the register.