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 --no-growable-memory #82890

Merged
merged 3 commits into from
Feb 25, 2024

Conversation

SingleAccretion
Copy link
Contributor

@SingleAccretion SingleAccretion commented Feb 24, 2024

We recently added --initial-heap - an option that allows one to up the initial memory size without the burden of having to know exactly how much is needed.

However, in the process of implementing support for this in Emscripten (emscripten-core/emscripten#21071), we have realized that --initial-heap cannot support the use-case of non-growable memories by itself, since with it we don't know what to set --max-memory to.

We have thus agreed to move the above work forward by introducing another option to the linker (see emscripten-core/emscripten#21071 (comment)), one that would allow users to explicitly specify they want a non-growable memory.

This change does this by introducing --no-growable-memory: an option that is mutally exclusive with --max-memory (for simplicity - we can also decide that it should override or be overridable by --max-memory. In Emscripten a similar mix of options results in --no-growable-memory taking precedence). The option specifies that the maximum memory size should be set to the initial memory size, effectively disallowing memory growth.

Closes #81932.

Copy link

github-actions bot commented Feb 24, 2024

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

With this option, clients (Emscripten) can implement non-growable
memories without knowing the amount of initial memory upfront.
@SingleAccretion SingleAccretion marked this pull request as ready for review February 24, 2024 17:29
@SingleAccretion
Copy link
Contributor Author

@sbc100

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 24, 2024

@llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-wasm

Author: None (SingleAccretion)

Changes

We recently added --initial-heap - an option that allows one to up the initial memory size without the burden of having to know exactly how much is needed.

However, in the process of implementing support for this in Emscripten (emscripten-core/emscripten#21071), we have realized that --initial-heap cannot support the use-case of non-growable memories by itself, since with it we don't know what to set --max-memory to.

We have thus agreed to move the above work forward by introducing another option to the linker (see emscripten-core/emscripten#21071 (comment)), one that would allow users to explicitly specify they want a non-growable memory.

This change does this by introducing --max-memory-growth: an option that is mutally exclusive with --max-memory (for simplicity - we can also decide that it should override or be overridable by --max-memory. In Emscripten a similar mix of options results in --max-memory-growth taking precedence). The option specifies how many pages will be dynamically allocatable: in the final WASM file, it will result in a Maximum field that is equal to Minimum + --max-memory-growth.

Thus, the Emscripten -sALLOW_MEMORY_GROWTH=0 behavior will be implementable by simply passing --max-memory-growth=0 (and not passing --max-memory).

I have also considered adding a boolean flag (--no-memory-growth), but this more general approach looks more consistent with the existing layout options at no real complexity cost.

Closes #81932.


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

7 Files Affected:

  • (modified) lld/docs/WebAssembly.rst (+4)
  • (modified) lld/test/wasm/data-layout.s (+32)
  • (modified) lld/test/wasm/large-memory.test (+8)
  • (modified) lld/wasm/Config.h (+1)
  • (modified) lld/wasm/Driver.cpp (+1)
  • (modified) lld/wasm/Options.td (+3)
  • (modified) lld/wasm/Writer.cpp (+32-12)
diff --git a/lld/docs/WebAssembly.rst b/lld/docs/WebAssembly.rst
index 3f554de46d38a7..ede78309ecfb1e 100644
--- a/lld/docs/WebAssembly.rst
+++ b/lld/docs/WebAssembly.rst
@@ -131,6 +131,10 @@ WebAssembly-specific options:
 
   Initial size of the linear memory. Default: the sum of stack, static data and heap sizes.
 
+.. option:: --max-memory-growth=<value>
+
+  Maximum size of memory that can be allocated dynamically. Default: unlimited.
+
 .. option:: --max-memory=<value>
 
   Maximum size of the linear memory. Default: unlimited.
diff --git a/lld/test/wasm/data-layout.s b/lld/test/wasm/data-layout.s
index 2a447aad622167..6e6a6cde747e2f 100644
--- a/lld/test/wasm/data-layout.s
+++ b/lld/test/wasm/data-layout.s
@@ -103,6 +103,38 @@ local_struct_internal_ptr:
 # CHECK-MAX-NEXT:         Minimum:         0x2
 # CHECK-MAX-NEXT:         Maximum:         0x2
 
+# RUN: wasm-ld --no-entry --initial-memory=327680 --max-memory-growth=0 \
+# RUN:     -o %t_max.wasm %t.hello32.o
+# RUN: obj2yaml %t_max.wasm | FileCheck %s -check-prefix=CHECK-MAX-GROWTH-ZERO
+
+# CHECK-MAX-GROWTH-ZERO:        - Type:            MEMORY
+# CHECK-MAX-GROWTH-ZERO-NEXT:     Memories:
+# CHECK-MAX-GROWTH-ZERO:           - Flags:           [ HAS_MAX ]
+# CHECK-MAX-GROWTH-ZERO:             Minimum:         0x5
+# CHECK-MAX-GROWTH-ZERO:             Maximum:         0x5
+
+# RUN: wasm-ld --initial-memory=196608 --max-memory-growth=262144 \
+# RUN:     --no-entry -o %t_max.wasm %t.hello32.o
+# RUN: obj2yaml %t_max.wasm | FileCheck %s -check-prefix=CHECK-MAX-GROWTH-SOME
+
+# CHECK-MAX-GROWTH-SOME:        - Type:            MEMORY
+# CHECK-MAX-GROWTH-SOME-NEXT:     Memories:
+# CHECK-MAX-GROWTH-SOME:           - Flags:           [ HAS_MAX ]
+# CHECK-MAX-GROWTH-SOME:             Minimum:         0x3
+# CHECK-MAX-GROWTH-SOME:             Maximum:         0x7
+
+# RUN: not wasm-ld --max-memory-growth=131073 \
+# RUN:     --no-entry -o %t_max.wasm %t.hello32.o 2>&1 \
+# RUN: | FileCheck %s --check-prefix CHECK-MAX-GROWTH-ALIGN-ERROR
+
+# CHECK-MAX-GROWTH-ALIGN-ERROR: maximum memory growth must be 65536-byte aligned
+
+# RUN: not wasm-ld --initial-memory=131072 --max-memory=262144 --max-memory-growth=131072 \
+# RUN:     --no-entry -o %t_max.wasm %t.hello32.o 2>&1 \
+# RUN: | FileCheck %s --check-prefix CHECK-MAX-GROWTH-COMPAT-ERROR
+
+# CHECK-MAX-GROWTH-COMPAT-ERROR: --max-memory-growth and --max-memory are mutually exclusive
+
 # RUN: wasm-ld -no-gc-sections --allow-undefined --no-entry --shared-memory \
 # RUN:     --features=atomics,bulk-memory --initial-memory=131072 \
 # RUN:     --max-memory=131072 -o %t_max.wasm %t32.o %t.hello32.o
diff --git a/lld/test/wasm/large-memory.test b/lld/test/wasm/large-memory.test
index 5b737e41549630..bc39a65bb3311b 100644
--- a/lld/test/wasm/large-memory.test
+++ b/lld/test/wasm/large-memory.test
@@ -5,10 +5,16 @@ RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %p/Inputs/start.s -o %
 RUN: wasm-ld %t.o -o %t1.wasm --max-memory=2147483648
 RUN: obj2yaml %t1.wasm | FileCheck %s --check-prefixes=CHECK,CHECK-2G
 
+RUN: wasm-ld %t.o -o %t1.wasm --initial-memory=131072 --max-memory-growth=2147352576
+RUN: obj2yaml %t1.wasm | FileCheck %s --check-prefixes=CHECK,CHECK-2G
+
 ; And also 4G of total memory
 RUN: wasm-ld %t.o -o %t2.wasm --max-memory=4294967296
 RUN: obj2yaml %t2.wasm | FileCheck %s --check-prefixes=CHECK,CHECK-4G
 
+RUN: wasm-ld %t.o -o %t2.wasm --initial-memory=131072 --max-memory-growth=4294836224
+RUN: obj2yaml %t2.wasm | FileCheck %s --check-prefixes=CHECK,CHECK-4G
+
 CHECK:      - Type:            MEMORY
 CHECK-NEXT:   Memories:
 CHECK-NEXT:     - Flags:           [ HAS_MAX ]
@@ -19,6 +25,8 @@ CHECK-4G-NEXT:    Maximum:         0x10000
 ; Test error for more than 4G of memory
 RUN: not wasm-ld %t.o -o %t3.wasm --initial-memory=4295032832 2>&1 | FileCheck %s --check-prefix INIT-ERROR
 RUN: not wasm-ld %t.o -o %t4.wasm --max-memory=4295032832 2>&1 | FileCheck %s --check-prefix MAX-ERROR
+RUN: not wasm-ld %t.o -o %t4.wasm --initial-memory=131072 --max-memory-growth=4294901760 2>&1 | FileCheck %s --check-prefix MAX-GROWTH-ERROR
 
 INIT-ERROR: initial memory too large, cannot be greater than 4294967296
 MAX-ERROR: maximum memory too large, cannot be greater than 4294967296
+MAX-GROWTH-ERROR: maximum memory growth too large, cannot be greater than 4294836224
diff --git a/lld/wasm/Config.h b/lld/wasm/Config.h
index 97c508bda6a1c3..70872685b40120 100644
--- a/lld/wasm/Config.h
+++ b/lld/wasm/Config.h
@@ -77,6 +77,7 @@ struct Configuration {
   uint64_t globalBase;
   uint64_t initialHeap;
   uint64_t initialMemory;
+  uint64_t maxMemoryGrowth;
   uint64_t maxMemory;
   // The table offset at which to place function addresses.  We reserve zero
   // for the null function pointer.  This gets set to 1 for executables and 0
diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp
index 635f19f78b15e6..87afdd223a707a 100644
--- a/lld/wasm/Driver.cpp
+++ b/lld/wasm/Driver.cpp
@@ -541,6 +541,7 @@ static void readConfigs(opt::InputArgList &args) {
   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->maxMemoryGrowth = args::getInteger(args, OPT_max_memory_growth, -1);
   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 8190717cef63bb..ed3e014850278a 100644
--- a/lld/wasm/Options.td
+++ b/lld/wasm/Options.td
@@ -227,6 +227,9 @@ def initial_heap: JJ<"initial-heap=">,
 def initial_memory: JJ<"initial-memory=">,
   HelpText<"Initial size of the linear memory">;
 
+def max_memory_growth: JJ<"max-memory-growth=">,
+  HelpText<"Maximum size of dynamically allocatable linear memory">;
+
 def max_memory: JJ<"max-memory=">,
   HelpText<"Maximum size of the linear memory">;
 
diff --git a/lld/wasm/Writer.cpp b/lld/wasm/Writer.cpp
index d1a06c9ac9c2ae..261d633f11b745 100644
--- a/lld/wasm/Writer.cpp
+++ b/lld/wasm/Writer.cpp
@@ -473,6 +473,25 @@ void Writer::layoutMemory() {
     WasmSym::heapEnd->setVA(memoryPtr);
   }
 
+  if (config->maxMemory != 0 && config->maxMemoryGrowth != -1) {
+    // Erroring out here is simpler than defining precedence rules.
+    error("--max-memory-growth and --max-memory are mutually exclusive");
+  }
+
+  uint64_t maxMemory = 0;
+  if (config->maxMemoryGrowth != -1) {
+    if (config->maxMemoryGrowth !=
+        alignTo(config->maxMemoryGrowth, WasmPageSize))
+      error("maximum memory growth must be " + Twine(WasmPageSize) +
+            "-byte aligned");
+    uint64_t maxMaxMemoryGrowth = maxMemorySetting - memoryPtr;
+    if (config->maxMemoryGrowth > maxMaxMemoryGrowth)
+      error("maximum memory growth too large, cannot be greater than " +
+            Twine(maxMaxMemoryGrowth));
+
+    maxMemory = memoryPtr + config->maxMemoryGrowth;
+  }
+
   if (config->maxMemory != 0) {
     if (config->maxMemory != alignTo(config->maxMemory, WasmPageSize))
       error("maximum memory must be " + Twine(WasmPageSize) + "-byte aligned");
@@ -481,20 +500,21 @@ void Writer::layoutMemory() {
     if (config->maxMemory > maxMemorySetting)
       error("maximum memory too large, cannot be greater than " +
             Twine(maxMemorySetting));
+
+    maxMemory = config->maxMemory;
   }
 
-  // Check max if explicitly supplied or required by shared memory
-  if (config->maxMemory != 0 || config->sharedMemory) {
-    uint64_t max = config->maxMemory;
-    if (max == 0) {
-      // If no maxMemory config was supplied but we are building with
-      // shared memory, we need to pick a sensible upper limit.
-      if (ctx.isPic)
-        max = maxMemorySetting;
-      else
-        max = memoryPtr;
-    }
-    out.memorySec->maxMemoryPages = max / WasmPageSize;
+  // If no maxMemory config was supplied but we are building with
+  // shared memory, we need to pick a sensible upper limit.
+  if (config->sharedMemory && maxMemory == 0) {
+    if (ctx.isPic)
+      maxMemory = maxMemorySetting;
+    else
+      maxMemory = memoryPtr;
+  }
+
+  if (maxMemory != 0) {
+    out.memorySec->maxMemoryPages = maxMemory / WasmPageSize;
     log("mem: max pages   = " + Twine(out.memorySec->maxMemoryPages));
   }
 }

@sbc100
Copy link
Collaborator

sbc100 commented Feb 24, 2024

How about adding --[no-]-growable-memory instead? It seems like a better match the existing --growable-table option and seem more obviously self-explanatory.

@SingleAccretion
Copy link
Contributor Author

SingleAccretion commented Feb 24, 2024

How about adding --[no-]-growable-memory instead?

I'll make the change. To be honest, I just wanted an option name that suggests it being related to --max-memory (since it is essentially a different way to specify that maximum memory), but it doesn't matter a great deal.

Edit: done.

@SingleAccretion SingleAccretion changed the title [lld][WebAssembly] Add --max-memory-growth [lld][WebAssembly] Add --no-growable-memory Feb 24, 2024
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.

lgtm with one more comment

lld/wasm/Writer.cpp Outdated Show resolved Hide resolved
@sbc100 sbc100 merged commit cb4f94d into llvm:main Feb 25, 2024
5 checks passed
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.

[wasm-ld] How to specify a non-growable memory without --initial-memory and --max-memory
3 participants