Skip to content

Commit b67917e

Browse files
authored
Fix alignment in MixedAllocator (#1740)
Necessary for simd, as we add a type with alignment >8. We were just broken on that before this PR.
1 parent 6b99d14 commit b67917e

File tree

3 files changed

+32
-21
lines changed

3 files changed

+32
-21
lines changed

src/emscripten-optimizer/simple_ast.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ class GlobalMixedArena : public MixedArena {
8080
public:
8181
template<class T>
8282
T* alloc() {
83-
auto* ret = static_cast<T*>(allocSpace(sizeof(T)));
83+
auto* ret = static_cast<T*>(allocSpace(sizeof(T), alignof(T)));
8484
new (ret) T();
8585
return ret;
8686
}
@@ -92,7 +92,7 @@ class ArrayStorage : public ArenaVectorBase<ArrayStorage, Ref> {
9292
public:
9393
void allocate(size_t size) {
9494
allocatedElements = size;
95-
data = static_cast<Ref*>(arena.allocSpace(sizeof(Ref) * allocatedElements));
95+
data = static_cast<Ref*>(arena.allocSpace(sizeof(Ref) * allocatedElements, alignof(Ref)));
9696
}
9797
};
9898

src/mixed_arena.h

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include <memory>
2424
#include <mutex>
2525
#include <thread>
26+
#include <type_traits>
2627
#include <vector>
2728

2829
//
@@ -58,9 +59,17 @@
5859

5960
struct MixedArena {
6061
// fast bump allocation
61-
std::vector<char*> chunks;
62-
size_t chunkSize = 32768;
63-
size_t index; // in last chunk
62+
63+
static const size_t CHUNK_SIZE = 32768;
64+
static const size_t MAX_ALIGN = 16; // allow 128bit SIMD
65+
66+
typedef std::aligned_storage<CHUNK_SIZE, MAX_ALIGN>::type Chunk;
67+
68+
// Each pointer in chunks is to an array of Chunk structs; typically 1,
69+
// but possibly more.
70+
std::vector<Chunk*> chunks;
71+
72+
size_t index = 0; // in last chunk
6473

6574
std::thread::id threadId;
6675

@@ -74,7 +83,8 @@ struct MixedArena {
7483
next.store(nullptr);
7584
}
7685

77-
void* allocSpace(size_t size) {
86+
// Allocate an amount of space with a guaranteed alignment
87+
void* allocSpace(size_t size, size_t align) {
7888
// the bump allocator data should not be modified by multiple threads at once.
7989
auto myId = std::this_thread::get_id();
8090
if (myId != threadId) {
@@ -104,32 +114,33 @@ struct MixedArena {
104114
curr = seen;
105115
}
106116
if (allocated) delete allocated;
107-
return curr->allocSpace(size);
108-
}
109-
size = (size + 7) & (-8); // same alignment as malloc TODO optimize?
110-
bool mustAllocate = false;
111-
while (chunkSize <= size) {
112-
chunkSize *= 2;
113-
mustAllocate = true;
117+
return curr->allocSpace(size, align);
114118
}
115-
if (chunks.size() == 0 || index + size >= chunkSize || mustAllocate) {
116-
chunks.push_back(new char[chunkSize]);
119+
// First, move the current index in the last chunk to an aligned position.
120+
index = (index + align - 1) & (-align);
121+
if (index + size > CHUNK_SIZE || chunks.size() == 0) {
122+
// Allocate a new chunk.
123+
auto numChunks = (size + CHUNK_SIZE - 1) / CHUNK_SIZE;
124+
assert(size <= numChunks * CHUNK_SIZE);
125+
chunks.push_back(new Chunk[numChunks]);
117126
index = 0;
118127
}
119-
auto* ret = chunks.back() + index;
120-
index += size;
128+
uint8_t* ret = static_cast<uint8_t*>(static_cast<void*>(chunks.back()));
129+
ret += index;
130+
index += size; // TODO: if we allocated more than 1 chunk, reuse the remainder, right now we allocate another next time
121131
return static_cast<void*>(ret);
122132
}
123133

124134
template<class T>
125135
T* alloc() {
126-
auto* ret = static_cast<T*>(allocSpace(sizeof(T)));
136+
static_assert(alignof(T) <= MAX_ALIGN, "maximum alignment not large enough");
137+
auto* ret = static_cast<T*>(allocSpace(sizeof(T), alignof(T)));
127138
new (ret) T(*this); // allocated objects receive the allocator, so they can allocate more later if necessary
128139
return ret;
129140
}
130141

131142
void clear() {
132-
for (char* chunk : chunks) {
143+
for (auto* chunk : chunks) {
133144
delete[] chunk;
134145
}
135146
chunks.clear();
@@ -324,7 +335,7 @@ class ArenaVector : public ArenaVectorBase<ArenaVector<T>, T> {
324335

325336
void allocate(size_t size) {
326337
this->allocatedElements = size;
327-
this->data = static_cast<T*>(allocator.allocSpace(sizeof(T) * this->allocatedElements));
338+
this->data = static_cast<T*>(allocator.allocSpace(sizeof(T) * this->allocatedElements, alignof(T)));
328339
}
329340
};
330341

src/wasm/wasm-s-parser.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ void SExpressionParser::parseDebugLocation() {
166166
return; // no column number
167167
}
168168
std::string colStr(++pos, debugLocEnd);
169-
void* buf = allocator.allocSpace(sizeof(SourceLocation));
169+
void* buf = allocator.allocSpace(sizeof(SourceLocation), alignof(SourceLocation));
170170
loc = new (buf) SourceLocation(IString(name.c_str(), false), atoi(lineStr.c_str()), atoi(colStr.c_str()));
171171
}
172172

0 commit comments

Comments
 (0)