Skip to content
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

[lld][WebAssembly] Add an --initial-heap option #75594

Merged
merged 1 commit into from
Dec 15, 2023

Conversation

SingleAccretion
Copy link
Contributor

@SingleAccretion SingleAccretion commented Dec 15, 2023

It is beneficial to preallocate a certain number of pages in the linear memory (i. e. use the "minimum" field of WASM memories) so that fewer "memory.grow"s are needed at startup.

So far, the way to do that has been to pass the "--initial-memory" option to the linker. It works, but has the very significant downside of requiring the user to know the size of static data beforehand, as it must not exceed the number of bytes passed-in as "--initial-memory".

The new "--initial-heap" option avoids this downside by simply appending the specified number of pages to static data (and stack), regardless of how large they already are.

Ref: emscripten-core/emscripten#20888.

Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

Copy link

github-actions bot commented Dec 15, 2023

:white_check_mark: With the latest revision this PR passed the C/C++ code formatter.

It is beneficial to preallocate a certain number of pages in the linear
memory (i. e. use the "minimum" field of WASM memories) so that fewer
"memory.grow"s are needed at startup.

So far, the way to do that has been to pass the "--initial-memory" option
to the linker. It works, but has the very significant downside of requiring
the user to know the size of static data beforehand, as it must not exceed
the number of bytes passed-in as "--initial-memory".

The new "--initial-heap" option avoids this downside by simply appending
the specified number of pages to static data (and stack), regardless of how
large they already are.
@SingleAccretion SingleAccretion marked this pull request as ready for review December 15, 2023 12:54
@llvmbot
Copy link
Member

llvmbot commented Dec 15, 2023

@llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-wasm

Author: None (SingleAccretion)

Changes

It is beneficial to preallocate a certain number of pages in the linear memory (i. e. use the "minimum" field of WASM memories) so that fewer "memory.grow"s are needed at startup.

So far, the way to do that has been to pass the "--initial-memory" option to the linker. It works, but has the very significant downside of requiring the user to know the size of static data beforehand, as it must not exceed the number of bytes passed-in as "--initial-memory".

The new "--initial-heap" option avoids this downside by simply appending the specified number of pages to static data (and stack), regardless of how large they already are.

Ref: emscripten-core/emscripten#20888.


Full diff: https://github.com/llvm/llvm-project/pull/75594.diff

6 Files Affected:

  • (modified) lld/docs/WebAssembly.rst (+5-1)
  • (added) lld/test/wasm/initial-heap.test (+27)
  • (modified) lld/wasm/Config.h (+1)
  • (modified) lld/wasm/Driver.cpp (+3-2)
  • (modified) lld/wasm/Options.td (+3)
  • (modified) lld/wasm/Writer.cpp (+10)
diff --git a/lld/docs/WebAssembly.rst b/lld/docs/WebAssembly.rst
index dad3177e2c7dff..3f554de46d38a7 100644
--- a/lld/docs/WebAssembly.rst
+++ b/lld/docs/WebAssembly.rst
@@ -123,9 +123,13 @@ WebAssembly-specific options:
    is not possible for undefined data symbols.  Undefined data symbols will
    still be reported as normal (in accordance with ``--unresolved-symbols``).
 
+.. option:: --initial-heap=<value>
+
+  Initial size of the heap. Default: zero.
+
 .. option:: --initial-memory=<value>
 
-  Initial size of the linear memory. Default: static data size.
+  Initial size of the linear memory. Default: the sum of stack, static data and heap sizes.
 
 .. option:: --max-memory=<value>
 
diff --git a/lld/test/wasm/initial-heap.test b/lld/test/wasm/initial-heap.test
new file mode 100644
index 00000000000000..3e8bbd36535d30
--- /dev/null
+++ b/lld/test/wasm/initial-heap.test
@@ -0,0 +1,27 @@
+RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %p/Inputs/start.s -o %t.o
+
+; The initial heap size will be added to the stack size
+RUN: wasm-ld %t.o -o %t1.wasm --stack-first -z stack-size=65536 --initial-heap=131072
+RUN: obj2yaml %t1.wasm | FileCheck %s --check-prefixes=CHECK,CHECK-2P
+
+; Also test that we can parse and process a large size correctly
+RUN: wasm-ld %t.o -o %t2.wasm --stack-first -z stack-size=65536 --initial-heap=4294901760
+RUN: obj2yaml %t2.wasm | FileCheck %s --check-prefixes=CHECK,CHECK-4G
+
+CHECK:      - Type:            MEMORY
+CHECK-NEXT:   Memories:
+CHECK-2P-NEXT:    Minimum:         0x3
+CHECK-4G-NEXT:    Minimum:         0x10000
+
+; Test various error cases.
+RUN: not wasm-ld %t.o -o %t3.wasm --initial-heap=131073 2>&1 | FileCheck %s --check-prefix NOT-PAGE-MULTIPLE
+RUN: not wasm-ld %t.o -o %t4.wasm --stack-first -z stack-size=65536 --initial-heap=4295032832 2>&1 | FileCheck %s --check-prefix TOO-LARGE-BY-ITSELF
+RUN: not wasm-ld %t.o -o %t5.wasm --stack-first -z stack-size=131072 --initial-heap=4294901760 2>&1 | FileCheck %s --check-prefix TOO-LARGE-WITH-STACK
+RUN: not wasm-ld %t.o -o %t6.wasm --stack-first -z stack-size=65536 --initial-heap=131072 --initial-memory=131072 2>&1 | FileCheck %s --check-prefix INITIAL-MEMORY-TOO-SMALL
+RUN: not wasm-ld %t.o -o %t7.wasm --stack-first -z stack-size=65536 --initial-heap=131072 --max-memory=131072 2>&1 | FileCheck %s --check-prefix MAX-MEMORY-TOO-SMALL
+
+NOT-PAGE-MULTIPLE: initial heap must be 65536-byte aligned
+TOO-LARGE-BY-ITSELF: initial heap too large, cannot be greater than 4294901760
+TOO-LARGE-WITH-STACK: initial heap too large, cannot be greater than 4294836224
+INITIAL-MEMORY-TOO-SMALL: initial memory too small, 196608 bytes needed
+MAX-MEMORY-TOO-SMALL: maximum memory too small, 196608 bytes needed
diff --git a/lld/wasm/Config.h b/lld/wasm/Config.h
index d76d43852acffe..104d4704840149 100644
--- a/lld/wasm/Config.h
+++ b/lld/wasm/Config.h
@@ -68,6 +68,7 @@ struct Configuration {
   bool isStatic = false;
   bool trace;
   uint64_t globalBase;
+  uint64_t initialHeap;
   uint64_t initialMemory;
   uint64_t maxMemory;
   uint64_t zStackSize;
diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp
index c68fe33a14e29d..a354260c605258 100644
--- a/lld/wasm/Driver.cpp
+++ b/lld/wasm/Driver.cpp
@@ -502,9 +502,10 @@ static void readConfigs(opt::InputArgList &args) {
   errorHandler().verbose = args.hasArg(OPT_verbose);
   LLVM_DEBUG(errorHandler().verbose = true);
 
-  config->initialMemory = args::getInteger(args, OPT_initial_memory, 0);
-  config->globalBase = args::getInteger(args, OPT_global_base, 0);
   config->tableBase = args::getInteger(args, OPT_table_base, 0);
+  config->globalBase = args::getInteger(args, OPT_global_base, 0);
+  config->initialHeap = args::getInteger(args, OPT_initial_heap, 0);
+  config->initialMemory = args::getInteger(args, OPT_initial_memory, 0);
   config->maxMemory = args::getInteger(args, OPT_max_memory, 0);
   config->zStackSize =
       args::getZOptionValue(args, OPT_z, "stack-size", WasmPageSize);
diff --git a/lld/wasm/Options.td b/lld/wasm/Options.td
index 2df6196d5e8ce6..95ebc202a45187 100644
--- a/lld/wasm/Options.td
+++ b/lld/wasm/Options.td
@@ -215,6 +215,9 @@ defm soname: Eq<"soname", "Set the module name in the generated name section">;
 def import_table: FF<"import-table">,
   HelpText<"Import function table from the environment">;
 
+def initial_heap: JJ<"initial-heap=">,
+  HelpText<"Initial size of the heap">;
+
 def initial_memory: JJ<"initial-memory=">,
   HelpText<"Initial size of the linear memory">;
 
diff --git a/lld/wasm/Writer.cpp b/lld/wasm/Writer.cpp
index 0576bf2907e49c..805018c58dccb4 100644
--- a/lld/wasm/Writer.cpp
+++ b/lld/wasm/Writer.cpp
@@ -439,6 +439,16 @@ void Writer::layoutMemory() {
     maxMemorySetting = 1ULL << 34;
   }
 
+  if (config->initialHeap != 0) {
+    if (config->initialHeap != alignTo(config->initialHeap, WasmPageSize))
+      error("initial heap must be " + Twine(WasmPageSize) + "-byte aligned");
+    uint64_t maxInitialHeap = maxMemorySetting - memoryPtr;
+    if (config->initialHeap > maxInitialHeap)
+      error("initial heap too large, cannot be greater than " +
+            Twine(maxInitialHeap));
+    memoryPtr += config->initialHeap;
+  }
+
   if (config->initialMemory != 0) {
     if (config->initialMemory != alignTo(config->initialMemory, WasmPageSize))
       error("initial memory must be " + Twine(WasmPageSize) + "-byte aligned");

Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

Thanks for taking the time to work on this.

This looks really good. I can't remember that last time I got a change this big that didn't warrant some feedback!

@sbc100 sbc100 merged commit b2cdf3c into llvm:main Dec 15, 2023
9 checks passed
@SingleAccretion SingleAccretion deleted the wasm-ld-init-heap branch January 26, 2024 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants