[PATCH] D42030: [WebAssembly] Define __heap_base global

[Nicholas Wilson via Phabricator via llvm-commits]

ncw added inline comments.


================
Comment at: wasm/Writer.cpp:38
 static constexpr int kStackAlignment = 16;
+static constexpr int kHeapAlignment = 4096;
 
----------------
sbc100 wrote:
> dschuff wrote:
> > Why 4096? Since wasm pages are 16k, is there some particular reason for it to be this as opposed to something else?
> No really any good reason...  should we just make it 16-byte aligned like the stack?  In which case there is not need to do any more alignedment since the heap will follow the stack region which is already 16-byte aligned.
> 
> Incidentally our musl limits.h currently defines PAGE_SIZE to 4k.. should we change it to 16k?
(I think Wasm pages are actually 64KiB: https://webassembly.github.io/spec/core/exec/runtime.html#page-size)

I //wouldn't// change the Musl PAGE_SIZE to match. (I looked into this when doing my Musl port, and Rich Felker added this as a review comment.) PAGE_SIZE doesn't have to match anything the CPU does, what it really has to match is what mmap does. So whatever alignment your mmap (and hence malloc) perform should be exposed via PAGE_SIZE. In my Musl, I chose 32KiB for the page size, and mmap works in those units, so that malloc rounds up "large" allocations (>100KiB) to the nearest libc page. It doesn't matter that grow_memory/current_memory Wasm instructions work in 64KiB units, the mmap implementation does the appropriate translation.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D42030