[lld][WebAssembly] Allow --import-memory to take single name #160409
Merged
+22
−9
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously it required a pair of `module` and `name`, e.g `--import-memory=mymodule,mymemory`. However, rather confusingly if you just specified `--import-memory=foo` it would set the module to `foo` and the name to empty string. This changes the interpretation of `--import-memory=foo` to mean import `foo` from the default module (which is currently `env`, but one day we make it configurable);
|
@llvm/pr-subscribers-lld Author: Sam Clegg (sbc100) ChangesPreviously it required a pair of However, rather confusingly if you just specified This changes the interpretation of Full diff: https://github.com/llvm/llvm-project/pull/160409.diff 2 Files Affected:
diff --git a/lld/test/wasm/memory-naming.test b/lld/test/wasm/memory-naming.test
index b4aabaeeac357..766d9cd59050b 100644
--- a/lld/test/wasm/memory-naming.test
+++ b/lld/test/wasm/memory-naming.test
@@ -65,6 +65,21 @@
# CHECK-IMPORT-NEXT: Index: 0
# CHECK-IMPORT-NEXT: - Type:
+# RUN:wasm-ld --import-memory=foo -o %t.import.wasm %t.start.o
+# RUN: obj2yaml %t.import.wasm | FileCheck -check-prefix=CHECK-IMPORT-DEFAULT %s
+
+# Verify that memory import module defaults to `env`, which is the default
+# module for all imports.
+
+# CHECK-IMPORT-DEFAULT: - Type: IMPORT
+# CHECK-IMPORT-DEFAULT-NEXT: Imports:
+# CHECK-IMPORT-DEFAULT-NEXT: - Module: env
+# CHECK-IMPORT-DEFAULT-NEXT: Field: foo
+# CHECK-IMPORT-DEFAULT-NEXT: Kind: MEMORY
+# CHECK-IMPORT-DEFAULT-NEXT: Memory:
+# CHECK-IMPORT-DEFAULT-NEXT: Minimum: 0x2
+# CHECK-IMPORT-DEFAULT-NEXT: - Type:
+
# RUN:wasm-ld --import-memory=foo,bar --export-memory=qux -o %t.both.wasm %t.start.o
# RUN: obj2yaml %t.both.wasm | FileCheck -check-prefix=CHECK-BOTH %s
diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp
index 6332f1a793b2a..9b85b6c00b26d 100644
--- a/lld/wasm/Driver.cpp
+++ b/lld/wasm/Driver.cpp
@@ -542,14 +542,13 @@ static void readConfigs(opt::InputArgList &args) {
ctx.arg.noinhibitExec = args.hasArg(OPT_noinhibit_exec);
if (args.hasArg(OPT_import_memory_with_name)) {
- ctx.arg.memoryImport =
- args.getLastArgValue(OPT_import_memory_with_name).split(",");
+ auto argValue = args.getLastArgValue(OPT_import_memory_with_name);
+ if (argValue.contains(','))
+ ctx.arg.memoryImport = argValue.split(",");
+ else
+ ctx.arg.memoryImport = {defaultModule, argValue};
} else if (args.hasArg(OPT_import_memory)) {
- ctx.arg.memoryImport =
- std::pair<llvm::StringRef, llvm::StringRef>(defaultModule, memoryName);
- } else {
- ctx.arg.memoryImport =
- std::optional<std::pair<llvm::StringRef, llvm::StringRef>>();
+ ctx.arg.memoryImport = {defaultModule, memoryName};
}
if (args.hasArg(OPT_export_memory_with_name)) {
@@ -746,8 +745,7 @@ static void setConfigs() {
error("--export-memory is incompatible with --shared");
}
if (!ctx.arg.memoryImport.has_value()) {
- ctx.arg.memoryImport = std::pair<llvm::StringRef, llvm::StringRef>(
- defaultModule, memoryName);
+ ctx.arg.memoryImport = {defaultModule, memoryName};
}
}
|
|
@llvm/pr-subscribers-lld-wasm Author: Sam Clegg (sbc100) ChangesPreviously it required a pair of However, rather confusingly if you just specified This changes the interpretation of Full diff: https://github.com/llvm/llvm-project/pull/160409.diff 2 Files Affected:
diff --git a/lld/test/wasm/memory-naming.test b/lld/test/wasm/memory-naming.test
index b4aabaeeac357..766d9cd59050b 100644
--- a/lld/test/wasm/memory-naming.test
+++ b/lld/test/wasm/memory-naming.test
@@ -65,6 +65,21 @@
# CHECK-IMPORT-NEXT: Index: 0
# CHECK-IMPORT-NEXT: - Type:
+# RUN:wasm-ld --import-memory=foo -o %t.import.wasm %t.start.o
+# RUN: obj2yaml %t.import.wasm | FileCheck -check-prefix=CHECK-IMPORT-DEFAULT %s
+
+# Verify that memory import module defaults to `env`, which is the default
+# module for all imports.
+
+# CHECK-IMPORT-DEFAULT: - Type: IMPORT
+# CHECK-IMPORT-DEFAULT-NEXT: Imports:
+# CHECK-IMPORT-DEFAULT-NEXT: - Module: env
+# CHECK-IMPORT-DEFAULT-NEXT: Field: foo
+# CHECK-IMPORT-DEFAULT-NEXT: Kind: MEMORY
+# CHECK-IMPORT-DEFAULT-NEXT: Memory:
+# CHECK-IMPORT-DEFAULT-NEXT: Minimum: 0x2
+# CHECK-IMPORT-DEFAULT-NEXT: - Type:
+
# RUN:wasm-ld --import-memory=foo,bar --export-memory=qux -o %t.both.wasm %t.start.o
# RUN: obj2yaml %t.both.wasm | FileCheck -check-prefix=CHECK-BOTH %s
diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp
index 6332f1a793b2a..9b85b6c00b26d 100644
--- a/lld/wasm/Driver.cpp
+++ b/lld/wasm/Driver.cpp
@@ -542,14 +542,13 @@ static void readConfigs(opt::InputArgList &args) {
ctx.arg.noinhibitExec = args.hasArg(OPT_noinhibit_exec);
if (args.hasArg(OPT_import_memory_with_name)) {
- ctx.arg.memoryImport =
- args.getLastArgValue(OPT_import_memory_with_name).split(",");
+ auto argValue = args.getLastArgValue(OPT_import_memory_with_name);
+ if (argValue.contains(','))
+ ctx.arg.memoryImport = argValue.split(",");
+ else
+ ctx.arg.memoryImport = {defaultModule, argValue};
} else if (args.hasArg(OPT_import_memory)) {
- ctx.arg.memoryImport =
- std::pair<llvm::StringRef, llvm::StringRef>(defaultModule, memoryName);
- } else {
- ctx.arg.memoryImport =
- std::optional<std::pair<llvm::StringRef, llvm::StringRef>>();
+ ctx.arg.memoryImport = {defaultModule, memoryName};
}
if (args.hasArg(OPT_export_memory_with_name)) {
@@ -746,8 +745,7 @@ static void setConfigs() {
error("--export-memory is incompatible with --shared");
}
if (!ctx.arg.memoryImport.has_value()) {
- ctx.arg.memoryImport = std::pair<llvm::StringRef, llvm::StringRef>(
- defaultModule, memoryName);
+ ctx.arg.memoryImport = {defaultModule, memoryName};
}
}
|
dschuff
approved these changes
Sep 23, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously we only tested it as taking a pair of
moduleandname, e.g--import-memory=mymodule,mymemory.However, rather confusingly, if you just specified
--import-memory=fooit would set the module tofooand the name to empty string.This changes the interpretation of
--import-memory=footo mean importfoofrom the default module (which is currentlyenv, but one day we make it configurable).