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

wasm-unknown target shouldn't set --import-memory by default #4319

Closed
jmillikin opened this issue Jun 30, 2024 · 3 comments
Closed

wasm-unknown target shouldn't set --import-memory by default #4319

jmillikin opened this issue Jun 30, 2024 · 3 comments
Labels
wasm WebAssembly

Comments

@jmillikin
Copy link

The wasm-unknown target invokes wasm-ld with the --import-memory option, which causes the following import to be emitted in the output:

(import "env" "memory" (memory $mimport$0 2))

This is undesirable to have as default behavior for wasm-unknown, because it introduces a mandatory import into what is otherwise a freestanding module.

If the --import-memory were to be removed from targets/wasm-unknown.json ldflags, then wasm-ld would emit this instead:

;; Declare and export the Memory, which requires no special import binding in the host
(memory $0 2)
(export "memory" (memory $0))

This option appears to have been added as part of a larger change in 2cd966d, but it's unclear from the commit message why --import-memory was added. Based on discussion in the PR, I think it was simply to reduce the number of bytes in an absolute bare-bones "hello world" module, which IMO is not a compelling reason to have it set by default.

@aykevl
Copy link
Member

aykevl commented Jun 30, 2024

Because wasm-unknown by default doesn't target a particular platform, it's difficult to say whether memory should be imported or exported by default.
@deadprogram what do environments like wazero expect? Memory, or no memory? Or do they support both?

@deadprogram
Copy link
Member

I think @jmillikin is probably correct in what the default should be.

@deadprogram
Copy link
Member

deadprogram commented Aug 16, 2024

See #4414 for a correction.

@deadprogram deadprogram added the next-release Will be part of next release label Aug 17, 2024
@deadprogram deadprogram removed the next-release Will be part of next release label Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasm WebAssembly
Projects
None yet
Development

No branches or pull requests

3 participants