[libc-commits] [libc] [libc] Lazily initialize freelist malloc using symbols (PR #99254)
Daniel Thornburgh via libc-commits
libc-commits at lists.llvm.org
Mon Jul 22 12:53:04 PDT 2024
https://github.com/mysterymath updated https://github.com/llvm/llvm-project/pull/99254
>From 3b8e715f82c2e0f43aab655af2e9f81181d98919 Mon Sep 17 00:00:00 2001
From: Daniel Thornburgh <dthorn at google.com>
Date: Tue, 16 Jul 2024 11:12:17 -0700
Subject: [PATCH 1/8] [libc] Lazily initialize freelist malloc using symbols
This allows the user to customize the size of the heap provided by
overriding the weak symbol __libc_heap_limit at link time. The heap
begins at _end and ends __libc_heap_limit bytes afterwards. It also
prevents a completely unused heap from requiring any space. (It's
reasonably common to link in malloc/free to handle exceptional
situations that can never actually be encountered.)
I'd think this should eventually be replaced with an implemenation based
on sbrk(), with sbrk() implemented generically in terms of brk(), which
would be a system call on e.g. Linux and a bump pointer from _end up to
__libc_heap_limit on baremetal. This would allow the allocator to be
more flexible (e.g., an implementation could swap out brk() and do
dynamic memory availability checks), to manage the heap better by
keeping better track of "wilderness" and to work properly as a malloc on
Linux.
Incidentally, this sets the default heap limit to 128KiB, from 102400
bytes, which had been reported as "1GiB".
---
libc/config/baremetal/config.json | 2 +-
libc/src/__support/freelist_heap.h | 75 ++++++++++---------
libc/src/stdlib/CMakeLists.txt | 1 +
libc/src/stdlib/freelist_malloc.cpp | 9 +--
libc/src/stdlib/heap_limit.S | 2 +
.../test/src/__support/freelist_heap_test.cpp | 7 +-
.../src/__support/freelist_malloc_test.cpp | 15 +++-
7 files changed, 63 insertions(+), 48 deletions(-)
create mode 100644 libc/src/stdlib/heap_limit.S
diff --git a/libc/config/baremetal/config.json b/libc/config/baremetal/config.json
index 12f4c2aa3a805..ebab661cb8d56 100644
--- a/libc/config/baremetal/config.json
+++ b/libc/config/baremetal/config.json
@@ -20,7 +20,7 @@
},
"malloc": {
"LIBC_CONF_FREELIST_MALLOC_BUFFER_SIZE": {
- "value": 102400
+ "value": 131072
}
},
"qsort": {
diff --git a/libc/src/__support/freelist_heap.h b/libc/src/__support/freelist_heap.h
index ce4f14b431585..31fca66b897b8 100644
--- a/libc/src/__support/freelist_heap.h
+++ b/libc/src/__support/freelist_heap.h
@@ -20,6 +20,9 @@
#include "src/string/memory_utils/inline_memcpy.h"
#include "src/string/memory_utils/inline_memset.h"
+extern char _end;
+extern char __libc_heap_limit;
+
namespace LIBC_NAMESPACE_DECL {
using cpp::optional;
@@ -47,17 +50,10 @@ template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()> class FreeListHeap {
size_t total_free_calls;
};
- FreeListHeap(span<cpp::byte> region)
- : FreeListHeap(&*region.begin(), &*region.end(), region.size()) {
- auto result = BlockType::init(region);
- BlockType *block = *result;
- freelist_.add_chunk(block_to_span(block));
- }
-
- constexpr FreeListHeap(void *start, cpp::byte *end, size_t total_bytes)
- : block_region_start_(start), block_region_end_(end),
- freelist_(DEFAULT_BUCKETS), heap_stats_{} {
- heap_stats_.total_bytes = total_bytes;
+ constexpr FreeListHeap(span<cpp::byte> region)
+ : is_initialized_(false), region_(region), freelist_(DEFAULT_BUCKETS),
+ heap_stats_{} {
+ heap_stats_.total_bytes = region.size();
}
void *allocate(size_t size);
@@ -69,61 +65,68 @@ template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()> class FreeListHeap {
void *calloc(size_t num, size_t size);
const HeapStats &heap_stats() const { return heap_stats_; }
- void reset_heap_stats() { heap_stats_ = {}; }
- void *region_start() const { return block_region_start_; }
- size_t region_size() const {
- return reinterpret_cast<uintptr_t>(block_region_end_) -
- reinterpret_cast<uintptr_t>(block_region_start_);
- }
+ cpp::span<cpp::byte> region() const { return region_; }
-protected:
- constexpr void set_freelist_node(typename FreeListType::FreeListNode &node,
- cpp::span<cpp::byte> chunk) {
- freelist_.set_freelist_node(node, chunk);
- }
+private:
+ void init();
void *allocate_impl(size_t alignment, size_t size);
-private:
span<cpp::byte> block_to_span(BlockType *block) {
return span<cpp::byte>(block->usable_space(), block->inner_size());
}
bool is_valid_ptr(void *ptr) {
- return ptr >= block_region_start_ && ptr < block_region_end_;
+ return ptr >= region_.begin() && ptr < region_.end();
}
- void *block_region_start_;
- void *block_region_end_;
+ bool is_initialized_;
+ cpp::span<cpp::byte> region_;
FreeListType freelist_;
HeapStats heap_stats_;
};
template <size_t BUFF_SIZE, size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()>
-struct FreeListHeapBuffer : public FreeListHeap<NUM_BUCKETS> {
+class FreeListHeapBuffer : public FreeListHeap<NUM_BUCKETS> {
using parent = FreeListHeap<NUM_BUCKETS>;
using FreeListNode = typename parent::FreeListType::FreeListNode;
+public:
constexpr FreeListHeapBuffer()
- : FreeListHeap<NUM_BUCKETS>(&block, buffer + sizeof(buffer), BUFF_SIZE),
- block(0, BUFF_SIZE), node{}, buffer{} {
- block.mark_last();
+ : FreeListHeap<NUM_BUCKETS>{buffer}, buffer{} {}
- cpp::span<cpp::byte> chunk(buffer, sizeof(buffer));
- parent::set_freelist_node(node, chunk);
- }
+private:
+ cpp::byte buffer[BUFF_SIZE];
+};
+
+template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()>
+class FreeListHeapSymbols : public FreeListHeap<NUM_BUCKETS> {
+ using parent = FreeListHeap<NUM_BUCKETS>;
+ using FreeListNode = typename parent::FreeListType::FreeListNode;
- typename parent::BlockType block;
- FreeListNode node;
- cpp::byte buffer[BUFF_SIZE - sizeof(block) - sizeof(node)];
+public:
+ constexpr FreeListHeapSymbols()
+ : FreeListHeap<NUM_BUCKETS>{
+ {(cpp::byte *)&_end, (size_t)&__libc_heap_limit}} {}
};
+template <size_t NUM_BUCKETS> void FreeListHeap<NUM_BUCKETS>::init() {
+ LIBC_ASSERT(!is_initialized_ && "duplicate initialization");
+ auto result = BlockType::init(region_);
+ BlockType *block = *result;
+ freelist_.add_chunk(block_to_span(block));
+ is_initialized_ = true;
+}
+
template <size_t NUM_BUCKETS>
void *FreeListHeap<NUM_BUCKETS>::allocate_impl(size_t alignment, size_t size) {
if (size == 0)
return nullptr;
+ if (!is_initialized_)
+ init();
+
// Find a chunk in the freelist. Split it if needed, then return.
auto chunk =
freelist_.find_chunk_if([alignment, size](span<cpp::byte> chunk) {
diff --git a/libc/src/stdlib/CMakeLists.txt b/libc/src/stdlib/CMakeLists.txt
index 513f6ad723d56..76f4a38091a91 100644
--- a/libc/src/stdlib/CMakeLists.txt
+++ b/libc/src/stdlib/CMakeLists.txt
@@ -381,6 +381,7 @@ if(NOT LIBC_TARGET_OS_IS_GPU)
freelist_malloc
SRCS
freelist_malloc.cpp
+ heap_limit.S
HDRS
malloc.h
DEPENDS
diff --git a/libc/src/stdlib/freelist_malloc.cpp b/libc/src/stdlib/freelist_malloc.cpp
index cfffa0425ff66..e15e74fb18864 100644
--- a/libc/src/stdlib/freelist_malloc.cpp
+++ b/libc/src/stdlib/freelist_malloc.cpp
@@ -19,16 +19,13 @@
namespace LIBC_NAMESPACE_DECL {
namespace {
-#ifdef LIBC_FREELIST_MALLOC_SIZE
-// This is set via the LIBC_CONF_FREELIST_MALLOC_BUFFER_SIZE configuration.
-constexpr size_t SIZE = LIBC_FREELIST_MALLOC_SIZE;
-#else
+#ifndef LIBC_FREELIST_MALLOC_SIZE
#error "LIBC_FREELIST_MALLOC_SIZE was not defined for this build."
#endif
-LIBC_CONSTINIT FreeListHeapBuffer<SIZE> freelist_heap_buffer;
+FreeListHeapSymbols<> freelist_heap_symbols;
} // namespace
-FreeListHeap<> *freelist_heap = &freelist_heap_buffer;
+FreeListHeap<> *freelist_heap = &freelist_heap_symbols;
LLVM_LIBC_FUNCTION(void *, malloc, (size_t size)) {
return freelist_heap->allocate(size);
diff --git a/libc/src/stdlib/heap_limit.S b/libc/src/stdlib/heap_limit.S
new file mode 100644
index 0000000000000..d704d07165ed9
--- /dev/null
+++ b/libc/src/stdlib/heap_limit.S
@@ -0,0 +1,2 @@
+.weak __libc_heap_limit
+.set __libc_heap_limit, LIBC_FREELIST_MALLOC_SIZE
diff --git a/libc/test/src/__support/freelist_heap_test.cpp b/libc/test/src/__support/freelist_heap_test.cpp
index 5815d5dfc01ab..fc4348aed6b56 100644
--- a/libc/test/src/__support/freelist_heap_test.cpp
+++ b/libc/test/src/__support/freelist_heap_test.cpp
@@ -30,6 +30,11 @@ using LIBC_NAMESPACE::freelist_heap;
#define TEST_FOR_EACH_ALLOCATOR(TestCase, BufferSize) \
class LlvmLibcFreeListHeapTest##TestCase : public testing::Test { \
public: \
+ FreeListHeapBuffer<BufferSize> fake_global_buffer; \
+ void SetUp() override { \
+ freelist_heap = \
+ new (&fake_global_buffer) FreeListHeapBuffer<BufferSize>; \
+ } \
void RunTest(FreeListHeap<> &allocator, [[maybe_unused]] size_t N); \
}; \
TEST_F(LlvmLibcFreeListHeapTest##TestCase, TestCase) { \
@@ -37,7 +42,7 @@ using LIBC_NAMESPACE::freelist_heap;
cpp::byte buf[BufferSize] = {cpp::byte(0)}; \
FreeListHeap<> allocator(buf); \
RunTest(allocator, BufferSize); \
- RunTest(*freelist_heap, freelist_heap->region_size()); \
+ RunTest(*freelist_heap, freelist_heap->region().size()); \
} \
void LlvmLibcFreeListHeapTest##TestCase::RunTest(FreeListHeap<> &allocator, \
size_t N)
diff --git a/libc/test/src/__support/freelist_malloc_test.cpp b/libc/test/src/__support/freelist_malloc_test.cpp
index e9d7c63a4d438..4b774884d81b5 100644
--- a/libc/test/src/__support/freelist_malloc_test.cpp
+++ b/libc/test/src/__support/freelist_malloc_test.cpp
@@ -14,15 +14,22 @@
#include "test/UnitTest/Test.h"
using LIBC_NAMESPACE::freelist_heap;
+using LIBC_NAMESPACE::FreeListHeapBuffer;
-TEST(LlvmLibcFreeListMalloc, MallocStats) {
+class LlvmLibcFreeListMalloc : public LIBC_NAMESPACE::testing::Test {
+protected:
+ void SetUp() override {
+ freelist_heap = new (&buffer) FreeListHeapBuffer<1024>;
+ }
+
+ FreeListHeapBuffer<1024> buffer;
+};
+
+TEST_F(LlvmLibcFreeListMalloc, MallocStats) {
constexpr size_t kAllocSize = 256;
constexpr size_t kCallocNum = 4;
constexpr size_t kCallocSize = 64;
- freelist_heap->reset_heap_stats(); // Do this because other tests might've
- // called the same global allocator.
-
void *ptr1 = LIBC_NAMESPACE::malloc(kAllocSize);
const auto &freelist_heap_stats = freelist_heap->heap_stats();
>From d2314dc1aa87e78f18324345f8bce6646a2c5ce6 Mon Sep 17 00:00:00 2001
From: Daniel Thornburgh <dthorn at google.com>
Date: Wed, 17 Jul 2024 09:59:12 -0700
Subject: [PATCH 2/8] Extern "C" in namespace for heap symbols
---
libc/src/__support/freelist_heap.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/libc/src/__support/freelist_heap.h b/libc/src/__support/freelist_heap.h
index 31fca66b897b8..8ca9003ff9724 100644
--- a/libc/src/__support/freelist_heap.h
+++ b/libc/src/__support/freelist_heap.h
@@ -20,11 +20,11 @@
#include "src/string/memory_utils/inline_memcpy.h"
#include "src/string/memory_utils/inline_memset.h"
-extern char _end;
-extern char __libc_heap_limit;
-
namespace LIBC_NAMESPACE_DECL {
+extern "C" char _end;
+extern "C" char __libc_heap_limit;
+
using cpp::optional;
using cpp::span;
>From 629141718ee057f1ace32f63dca54ba7f6fef1dc Mon Sep 17 00:00:00 2001
From: Daniel Thornburgh <dthorn at google.com>
Date: Wed, 17 Jul 2024 14:52:48 -0700
Subject: [PATCH 3/8] Address comments:
- Move preprocessor check to assembly file
- Add LIBC_CONSTINIT back to heap definition; make non-constexpr cast
from &__heap_limit to size_t occur at init time to restore constexpr
init.
---
libc/src/__support/freelist_heap.h | 32 +++++++++++++++--------------
libc/src/stdlib/freelist_malloc.cpp | 8 +-------
libc/src/stdlib/heap_limit.S | 3 +++
3 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/libc/src/__support/freelist_heap.h b/libc/src/__support/freelist_heap.h
index 8ca9003ff9724..463ac6a1fd591 100644
--- a/libc/src/__support/freelist_heap.h
+++ b/libc/src/__support/freelist_heap.h
@@ -22,8 +22,8 @@
namespace LIBC_NAMESPACE_DECL {
-extern "C" char _end;
-extern "C" char __libc_heap_limit;
+extern "C" cpp::byte _end;
+extern "C" cpp::byte __libc_heap_limit;
using cpp::optional;
using cpp::span;
@@ -50,9 +50,14 @@ template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()> class FreeListHeap {
size_t total_free_calls;
};
+ constexpr FreeListHeap()
+ : is_initialized_(false), region_(&_end, size_t{0}),
+ heap_limit_(&__libc_heap_limit), freelist_(DEFAULT_BUCKETS),
+ heap_stats_{} {}
+
constexpr FreeListHeap(span<cpp::byte> region)
- : is_initialized_(false), region_(region), freelist_(DEFAULT_BUCKETS),
- heap_stats_{} {
+ : is_initialized_(false), region_(region), heap_limit_{},
+ freelist_(DEFAULT_BUCKETS), heap_stats_{} {
heap_stats_.total_bytes = region.size();
}
@@ -83,6 +88,10 @@ template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()> class FreeListHeap {
bool is_initialized_;
cpp::span<cpp::byte> region_;
+
+ // Kept to initialize region_ by non-constexpr cast to size_t
+ cpp::byte *heap_limit_;
+
FreeListType freelist_;
HeapStats heap_stats_;
};
@@ -100,19 +109,12 @@ class FreeListHeapBuffer : public FreeListHeap<NUM_BUCKETS> {
cpp::byte buffer[BUFF_SIZE];
};
-template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()>
-class FreeListHeapSymbols : public FreeListHeap<NUM_BUCKETS> {
- using parent = FreeListHeap<NUM_BUCKETS>;
- using FreeListNode = typename parent::FreeListType::FreeListNode;
-
-public:
- constexpr FreeListHeapSymbols()
- : FreeListHeap<NUM_BUCKETS>{
- {(cpp::byte *)&_end, (size_t)&__libc_heap_limit}} {}
-};
-
template <size_t NUM_BUCKETS> void FreeListHeap<NUM_BUCKETS>::init() {
LIBC_ASSERT(!is_initialized_ && "duplicate initialization");
+ if (heap_limit_) {
+ region_ = {region_.begin(), (size_t)heap_limit_};
+ heap_stats_.total_bytes = region_.size();
+ }
auto result = BlockType::init(region_);
BlockType *block = *result;
freelist_.add_chunk(block_to_span(block));
diff --git a/libc/src/stdlib/freelist_malloc.cpp b/libc/src/stdlib/freelist_malloc.cpp
index e15e74fb18864..47240bc53aa37 100644
--- a/libc/src/stdlib/freelist_malloc.cpp
+++ b/libc/src/stdlib/freelist_malloc.cpp
@@ -18,13 +18,7 @@
namespace LIBC_NAMESPACE_DECL {
-namespace {
-#ifndef LIBC_FREELIST_MALLOC_SIZE
-#error "LIBC_FREELIST_MALLOC_SIZE was not defined for this build."
-#endif
-FreeListHeapSymbols<> freelist_heap_symbols;
-} // namespace
-
+static LIBC_CONSTINIT FreeListHeap<> freelist_heap_symbols;
FreeListHeap<> *freelist_heap = &freelist_heap_symbols;
LLVM_LIBC_FUNCTION(void *, malloc, (size_t size)) {
diff --git a/libc/src/stdlib/heap_limit.S b/libc/src/stdlib/heap_limit.S
index d704d07165ed9..2a0a6665deb56 100644
--- a/libc/src/stdlib/heap_limit.S
+++ b/libc/src/stdlib/heap_limit.S
@@ -1,2 +1,5 @@
+#ifndef LIBC_FREELIST_MALLOC_SIZE
+#error "LIBC_FREELIST_MALLOC_SIZE was not defined for this build."
+#endif
.weak __libc_heap_limit
.set __libc_heap_limit, LIBC_FREELIST_MALLOC_SIZE
>From e4cf6b3f6e09cad1013cc4dbaf1d7006499f810b Mon Sep 17 00:00:00 2001
From: Daniel Thornburgh <dthorn at google.com>
Date: Wed, 17 Jul 2024 15:40:46 -0700
Subject: [PATCH 4/8] Default initializers to clean up constructors
---
libc/src/__support/freelist_heap.h | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/libc/src/__support/freelist_heap.h b/libc/src/__support/freelist_heap.h
index 463ac6a1fd591..ee448c04cfb22 100644
--- a/libc/src/__support/freelist_heap.h
+++ b/libc/src/__support/freelist_heap.h
@@ -51,13 +51,10 @@ template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()> class FreeListHeap {
};
constexpr FreeListHeap()
- : is_initialized_(false), region_(&_end, size_t{0}),
- heap_limit_(&__libc_heap_limit), freelist_(DEFAULT_BUCKETS),
- heap_stats_{} {}
+ : region_(&_end, size_t{0}), heap_limit_(&__libc_heap_limit) {}
constexpr FreeListHeap(span<cpp::byte> region)
- : is_initialized_(false), region_(region), heap_limit_{},
- freelist_(DEFAULT_BUCKETS), heap_stats_{} {
+ : region_(region), heap_limit_{} {
heap_stats_.total_bytes = region.size();
}
@@ -86,14 +83,14 @@ template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()> class FreeListHeap {
return ptr >= region_.begin() && ptr < region_.end();
}
- bool is_initialized_;
+ bool is_initialized_ = false;
cpp::span<cpp::byte> region_;
// Kept to initialize region_ by non-constexpr cast to size_t
cpp::byte *heap_limit_;
- FreeListType freelist_;
- HeapStats heap_stats_;
+ FreeListType freelist_{DEFAULT_BUCKETS};
+ HeapStats heap_stats_{};
};
template <size_t BUFF_SIZE, size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()>
>From 6781b40863de8c3ff00bc7648a0619c9d3bf251a Mon Sep 17 00:00:00 2001
From: Daniel Thornburgh <dthorn at google.com>
Date: Fri, 19 Jul 2024 15:05:36 -0700
Subject: [PATCH 5/8] Make __libc_heap_limit an end symbol, not a size
This is more conventional and it dodges the issue of needing to provide
a default size entirely. Setting up a baremetal heap intrinsically
requires some linker script work, and __libc_heap_limit will be an
undefined reference hinting to the user that this work must be done.
Once we have a morecore function, a baremetal morecore() would use
__libc_heap_limit, a Linux one would use sbrk(), and a test one would
use a buffer.
---
libc/config/baremetal/config.json | 5 ----
libc/config/config.json | 6 -----
libc/docs/configure.rst | 2 --
libc/src/__support/freelist_heap.h | 26 +++++++--------------
libc/src/stdlib/CMakeLists.txt | 3 ---
libc/src/stdlib/heap_limit.S | 5 ----
libc/test/src/__support/CMakeLists.txt | 1 +
libc/test/src/__support/heap_limit_fake.cpp | 20 ++++++++++++++++
8 files changed, 29 insertions(+), 39 deletions(-)
delete mode 100644 libc/src/stdlib/heap_limit.S
create mode 100644 libc/test/src/__support/heap_limit_fake.cpp
diff --git a/libc/config/baremetal/config.json b/libc/config/baremetal/config.json
index ebab661cb8d56..f0ff268b56275 100644
--- a/libc/config/baremetal/config.json
+++ b/libc/config/baremetal/config.json
@@ -18,11 +18,6 @@
"value": false
}
},
- "malloc": {
- "LIBC_CONF_FREELIST_MALLOC_BUFFER_SIZE": {
- "value": 131072
- }
- },
"qsort": {
"LIBC_CONF_QSORT_IMPL": {
"value": "LIBC_QSORT_HEAP_SORT"
diff --git a/libc/config/config.json b/libc/config/config.json
index 2005f4297bfc1..178cedba6056a 100644
--- a/libc/config/config.json
+++ b/libc/config/config.json
@@ -71,12 +71,6 @@
"doc": "Default number of spins before blocking if a rwlock is in contention (default to 100)."
}
},
- "malloc": {
- "LIBC_CONF_FREELIST_MALLOC_BUFFER_SIZE": {
- "value": 1073741824,
- "doc": "Default size for the constinit freelist buffer used for the freelist malloc implementation (default 1o 1GB)."
- }
- },
"unistd": {
"LIBC_CONF_ENABLE_TID_CACHE": {
"value": true,
diff --git a/libc/docs/configure.rst b/libc/docs/configure.rst
index 5c55e4ab0f181..151ecd0d23a3e 100644
--- a/libc/docs/configure.rst
+++ b/libc/docs/configure.rst
@@ -30,8 +30,6 @@ to learn about the defaults for your platform and target.
- ``LIBC_CONF_KEEP_FRAME_POINTER``: Keep frame pointer in functions for better debugging experience.
* **"errno" options**
- ``LIBC_CONF_ERRNO_MODE``: The implementation used for errno, acceptable values are LIBC_ERRNO_MODE_UNDEFINED, LIBC_ERRNO_MODE_THREAD_LOCAL, LIBC_ERRNO_MODE_SHARED, LIBC_ERRNO_MODE_EXTERNAL, and LIBC_ERRNO_MODE_SYSTEM.
-* **"malloc" options**
- - ``LIBC_CONF_FREELIST_MALLOC_BUFFER_SIZE``: Default size for the constinit freelist buffer used for the freelist malloc implementation (default 1o 1GB).
* **"math" options**
- ``LIBC_CONF_MATH_OPTIMIZATIONS``: Configures optimizations for math functions. Values accepted are LIBC_MATH_SKIP_ACCURATE_PASS, LIBC_MATH_SMALL_TABLES, LIBC_MATH_NO_ERRNO, LIBC_MATH_NO_EXCEPT, and LIBC_MATH_FAST.
* **"printf" options**
diff --git a/libc/src/__support/freelist_heap.h b/libc/src/__support/freelist_heap.h
index ee448c04cfb22..dccfd753cc45f 100644
--- a/libc/src/__support/freelist_heap.h
+++ b/libc/src/__support/freelist_heap.h
@@ -50,12 +50,10 @@ template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()> class FreeListHeap {
size_t total_free_calls;
};
- constexpr FreeListHeap()
- : region_(&_end, size_t{0}), heap_limit_(&__libc_heap_limit) {}
+ constexpr FreeListHeap() : begin_(&_end), end_(&__libc_heap_limit) {}
constexpr FreeListHeap(span<cpp::byte> region)
- : region_(region), heap_limit_{} {
- heap_stats_.total_bytes = region.size();
+ : begin_(region.begin()), end_(region.end()) {
}
void *allocate(size_t size);
@@ -68,7 +66,7 @@ template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()> class FreeListHeap {
const HeapStats &heap_stats() const { return heap_stats_; }
- cpp::span<cpp::byte> region() const { return region_; }
+ cpp::span<cpp::byte> region() const { return {begin_, end_}; }
private:
void init();
@@ -79,16 +77,11 @@ template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()> class FreeListHeap {
return span<cpp::byte>(block->usable_space(), block->inner_size());
}
- bool is_valid_ptr(void *ptr) {
- return ptr >= region_.begin() && ptr < region_.end();
- }
+ bool is_valid_ptr(void *ptr) { return ptr >= begin_ && ptr < end_; }
bool is_initialized_ = false;
- cpp::span<cpp::byte> region_;
-
- // Kept to initialize region_ by non-constexpr cast to size_t
- cpp::byte *heap_limit_;
-
+ cpp::byte *begin_;
+ cpp::byte *end_;
FreeListType freelist_{DEFAULT_BUCKETS};
HeapStats heap_stats_{};
};
@@ -108,11 +101,8 @@ class FreeListHeapBuffer : public FreeListHeap<NUM_BUCKETS> {
template <size_t NUM_BUCKETS> void FreeListHeap<NUM_BUCKETS>::init() {
LIBC_ASSERT(!is_initialized_ && "duplicate initialization");
- if (heap_limit_) {
- region_ = {region_.begin(), (size_t)heap_limit_};
- heap_stats_.total_bytes = region_.size();
- }
- auto result = BlockType::init(region_);
+ heap_stats_.total_bytes = region().size();
+ auto result = BlockType::init(region());
BlockType *block = *result;
freelist_.add_chunk(block_to_span(block));
is_initialized_ = true;
diff --git a/libc/src/stdlib/CMakeLists.txt b/libc/src/stdlib/CMakeLists.txt
index 76f4a38091a91..52c1eb2606d08 100644
--- a/libc/src/stdlib/CMakeLists.txt
+++ b/libc/src/stdlib/CMakeLists.txt
@@ -381,13 +381,10 @@ if(NOT LIBC_TARGET_OS_IS_GPU)
freelist_malloc
SRCS
freelist_malloc.cpp
- heap_limit.S
HDRS
malloc.h
DEPENDS
libc.src.__support.freelist_heap
- COMPILE_OPTIONS
- -DLIBC_FREELIST_MALLOC_SIZE=${LIBC_CONF_FREELIST_MALLOC_BUFFER_SIZE}
)
get_target_property(freelist_malloc_is_skipped libc.src.stdlib.freelist_malloc "SKIPPED")
if(LIBC_TARGET_OS_IS_BAREMETAL AND NOT freelist_malloc_is_skipped)
diff --git a/libc/src/stdlib/heap_limit.S b/libc/src/stdlib/heap_limit.S
deleted file mode 100644
index 2a0a6665deb56..0000000000000
--- a/libc/src/stdlib/heap_limit.S
+++ /dev/null
@@ -1,5 +0,0 @@
-#ifndef LIBC_FREELIST_MALLOC_SIZE
-#error "LIBC_FREELIST_MALLOC_SIZE was not defined for this build."
-#endif
-.weak __libc_heap_limit
-.set __libc_heap_limit, LIBC_FREELIST_MALLOC_SIZE
diff --git a/libc/test/src/__support/CMakeLists.txt b/libc/test/src/__support/CMakeLists.txt
index f994f65e08cbb..7fb6fe2c84846 100644
--- a/libc/test/src/__support/CMakeLists.txt
+++ b/libc/test/src/__support/CMakeLists.txt
@@ -36,6 +36,7 @@ if(LLVM_LIBC_FULL_BUILD)
SRCS
freelist_heap_test.cpp
freelist_malloc_test.cpp
+ heap_limit_fake.cpp
DEPENDS
libc.src.__support.CPP.span
libc.src.__support.freelist_heap
diff --git a/libc/test/src/__support/heap_limit_fake.cpp b/libc/test/src/__support/heap_limit_fake.cpp
new file mode 100644
index 0000000000000..a03174ef7bdb7
--- /dev/null
+++ b/libc/test/src/__support/heap_limit_fake.cpp
@@ -0,0 +1,20 @@
+//===-- Test fake definition for __libc_heaplimit -------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/__support/CPP/cstddef.h"
+#include "src/__support/macros/config.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+extern "C" {
+// This isn't used in the unit tests, but it must be defined for the non-fake
+// version of the heap to work.
+cpp::byte __libc_heap_limit;
+}
+
+} // namespace LIBC_NAMESPACE_DECL
>From fbdd459fcc38453ced9b1c6893930094acaa9925 Mon Sep 17 00:00:00 2001
From: Daniel Thornburgh <dthorn at google.com>
Date: Fri, 19 Jul 2024 16:10:50 -0700
Subject: [PATCH 6/8] Address review comments
- __libc_heap_limit -> __llvm_libc_heap_limit
---
libc/src/__support/freelist_heap.h | 4 ++--
libc/test/src/__support/heap_limit_fake.cpp | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/libc/src/__support/freelist_heap.h b/libc/src/__support/freelist_heap.h
index dccfd753cc45f..d76c3d93dbf6e 100644
--- a/libc/src/__support/freelist_heap.h
+++ b/libc/src/__support/freelist_heap.h
@@ -23,7 +23,7 @@
namespace LIBC_NAMESPACE_DECL {
extern "C" cpp::byte _end;
-extern "C" cpp::byte __libc_heap_limit;
+extern "C" cpp::byte __llvm_libc_heap_limit;
using cpp::optional;
using cpp::span;
@@ -50,7 +50,7 @@ template <size_t NUM_BUCKETS = DEFAULT_BUCKETS.size()> class FreeListHeap {
size_t total_free_calls;
};
- constexpr FreeListHeap() : begin_(&_end), end_(&__libc_heap_limit) {}
+ constexpr FreeListHeap() : begin_(&_end), end_(&__llvm_libc_heap_limit) {}
constexpr FreeListHeap(span<cpp::byte> region)
: begin_(region.begin()), end_(region.end()) {
diff --git a/libc/test/src/__support/heap_limit_fake.cpp b/libc/test/src/__support/heap_limit_fake.cpp
index a03174ef7bdb7..6be79880c45c5 100644
--- a/libc/test/src/__support/heap_limit_fake.cpp
+++ b/libc/test/src/__support/heap_limit_fake.cpp
@@ -1,4 +1,4 @@
-//===-- Test fake definition for __libc_heaplimit -------------------------===//
+//===-- Test fake definition for __llvm_libc_heaplimit --------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -14,7 +14,7 @@ namespace LIBC_NAMESPACE_DECL {
extern "C" {
// This isn't used in the unit tests, but it must be defined for the non-fake
// version of the heap to work.
-cpp::byte __libc_heap_limit;
+cpp::byte __llvm_libc_heap_limit;
}
} // namespace LIBC_NAMESPACE_DECL
>From 57bfe5cbabc43620e07931a31339d5a64307aef2 Mon Sep 17 00:00:00 2001
From: Daniel Thornburgh <dthorn at google.com>
Date: Mon, 22 Jul 2024 12:32:11 -0700
Subject: [PATCH 7/8] Include fake for _end too.
---
libc/test/src/__support/heap_limit_fake.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libc/test/src/__support/heap_limit_fake.cpp b/libc/test/src/__support/heap_limit_fake.cpp
index 6be79880c45c5..82405499c1f5d 100644
--- a/libc/test/src/__support/heap_limit_fake.cpp
+++ b/libc/test/src/__support/heap_limit_fake.cpp
@@ -12,8 +12,9 @@
namespace LIBC_NAMESPACE_DECL {
extern "C" {
-// This isn't used in the unit tests, but it must be defined for the non-fake
+// These aren't used in the unit tests, but they must be defined for the non-fake
// version of the heap to work.
+cpp::byte _end;
cpp::byte __llvm_libc_heap_limit;
}
>From 5696d2f30f30a4fb34f05d982a2cbf91f4c468a1 Mon Sep 17 00:00:00 2001
From: Daniel Thornburgh <dthorn at google.com>
Date: Mon, 22 Jul 2024 12:52:29 -0700
Subject: [PATCH 8/8] Define _end and __llvm_libc_heap_limit to fake buffer for
malloc test
---
libc/test/src/__support/CMakeLists.txt | 2 +-
libc/test/src/__support/fake_heap.s | 15 +++++++++++++
.../src/__support/freelist_malloc_test.cpp | 11 +---------
libc/test/src/__support/heap_limit_fake.cpp | 21 -------------------
4 files changed, 17 insertions(+), 32 deletions(-)
create mode 100644 libc/test/src/__support/fake_heap.s
delete mode 100644 libc/test/src/__support/heap_limit_fake.cpp
diff --git a/libc/test/src/__support/CMakeLists.txt b/libc/test/src/__support/CMakeLists.txt
index 7fb6fe2c84846..90de520405981 100644
--- a/libc/test/src/__support/CMakeLists.txt
+++ b/libc/test/src/__support/CMakeLists.txt
@@ -34,9 +34,9 @@ if(LLVM_LIBC_FULL_BUILD)
SUITE
libc-support-tests
SRCS
+ fake_heap.s
freelist_heap_test.cpp
freelist_malloc_test.cpp
- heap_limit_fake.cpp
DEPENDS
libc.src.__support.CPP.span
libc.src.__support.freelist_heap
diff --git a/libc/test/src/__support/fake_heap.s b/libc/test/src/__support/fake_heap.s
new file mode 100644
index 0000000000000..69522f53c8b1f
--- /dev/null
+++ b/libc/test/src/__support/fake_heap.s
@@ -0,0 +1,15 @@
+//===-- Test fake definition for heap symbols -----------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+.globl _end, __llvm_libc_heap_limit
+
+.bss
+_end:
+.fill 1024
+__llvm_libc_heap_limit:
+
diff --git a/libc/test/src/__support/freelist_malloc_test.cpp b/libc/test/src/__support/freelist_malloc_test.cpp
index 4b774884d81b5..66a923f778218 100644
--- a/libc/test/src/__support/freelist_malloc_test.cpp
+++ b/libc/test/src/__support/freelist_malloc_test.cpp
@@ -16,16 +16,7 @@
using LIBC_NAMESPACE::freelist_heap;
using LIBC_NAMESPACE::FreeListHeapBuffer;
-class LlvmLibcFreeListMalloc : public LIBC_NAMESPACE::testing::Test {
-protected:
- void SetUp() override {
- freelist_heap = new (&buffer) FreeListHeapBuffer<1024>;
- }
-
- FreeListHeapBuffer<1024> buffer;
-};
-
-TEST_F(LlvmLibcFreeListMalloc, MallocStats) {
+TEST(LlvmLibcFreeListMalloc, MallocStats) {
constexpr size_t kAllocSize = 256;
constexpr size_t kCallocNum = 4;
constexpr size_t kCallocSize = 64;
diff --git a/libc/test/src/__support/heap_limit_fake.cpp b/libc/test/src/__support/heap_limit_fake.cpp
deleted file mode 100644
index 82405499c1f5d..0000000000000
--- a/libc/test/src/__support/heap_limit_fake.cpp
+++ /dev/null
@@ -1,21 +0,0 @@
-//===-- Test fake definition for __llvm_libc_heaplimit --------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "src/__support/CPP/cstddef.h"
-#include "src/__support/macros/config.h"
-
-namespace LIBC_NAMESPACE_DECL {
-
-extern "C" {
-// These aren't used in the unit tests, but they must be defined for the non-fake
-// version of the heap to work.
-cpp::byte _end;
-cpp::byte __llvm_libc_heap_limit;
-}
-
-} // namespace LIBC_NAMESPACE_DECL
More information about the libc-commits
mailing list