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

Ergonomics for codegen #169

Open
icefoxen opened this issue Jun 24, 2020 · 4 comments
Open

Ergonomics for codegen #169

icefoxen opened this issue Jun 24, 2020 · 4 comments
Labels
enhancement New feature or request

Comments

@icefoxen
Copy link

Motivation

Hi again, I'm continuing my use-walrus-as-a-compiler-backend adventure, and there's a couple things that have caused me some grief. After thinking about it more it feels like this is because walrus is currently mostly designed to take some existing wasm module as input and do stuff to it, and less to generate these modules from whole cloth. However, it's currently still the best crate available for generating wasm, as unlike most it a) is actually up to date and feels like the developers actually use it, and to a lesser extent b) is really nice because it does a lot of the accounting of identifiers and stuff for you.

However, this accounting has also caused me grief a few times. Most notably:

  • When generating functions, I need to be able to generate FunctionId's before I have a body for the function, for example to do forward references. If foo() calls bar() and bar() isn't yet defined, it's a lot easier for the compiler to do a pre-population pass through the code and say "I know about bar(), here's the type and FunctionId for it" than it is to then go back and fix up every reference to bar() with a real address/FunctionId. But the only way to do that I've found is to create a FunctionBuilder, finish it immediately to get the FunctionId, and then re-acquire the FunctionBuilder from the Function and carry on actually generating code for it... but the FunctionBuilder then mutably borrows part of the Module, and working around that gets rather painful rather fast. Possible, but painful. Other ID types tend to have the same problem to a lesser extent.
  • Making a Table and populating it with functions is a pain, mainly just 'cause you're doing everything by hand and have to juggle two or three different sections -- functions, tables and elements. I expect creating memory and data segments to be similarly annoying. Having a builder type or such for those would make life a lot easier.

Proposed Solution

I'd happily try to make a PR for a TableBuilder and MemoryBuilder type, if you wanted. Seems easy enough, even if I have no idea what ElementKind actually is -- can't find any references to that in the wasm spec, so I assume it's newer stuff that hasn't been finalized yet. My existing code is halfway there already.

As for the function stuff, I'm not sure what the best course of action is. One could make a function to reserve a FunctionId and then use it later to actually build the function. You could make it possible to kindasorta "disconnect" a FunctionBuilder from a particular Function, changing the reference into an Rc or something, or perhaps more simply just make it possible to create a Function, then later replace its body with a new, fresh FunctionBuilder you've created. Actually this might be possible with InstrSeq, but I don't quite understand how that portion of walrus works yet.

Alternatives

Don't do this, I guess? This is work I need to do anyway, so I might as well design it to be upstreamed if you guys want it. I could contribute some extra docs or examples if you prefer, since the existing ones are a little minimal.

@icefoxen icefoxen added the enhancement New feature or request label Jun 24, 2020
@alexcrichton
Copy link
Collaborator

Sorry for the delay, but PRs for these would definitely be welcome!

FWIW ElementKind mostly has to do with the reference types and bulk memory proposal, but you can largely ignore it most likel. Otherwise I think whatever APIs make sense here should be good to add!

@NicholasLYang
Copy link

Happy to work on this (exploring using walrus as a compiler backend as well). There's already a FunctionKind::Uninitialized, so we can use that. It could be as simple as adding a ModuleFunctions::add_uninitialized that takes in a type id and adds an uninitialized function to the arena. Then ModuleFunctions::get_mut should handle the swapping.

We will have to emit a proper error on trying to emit an uninitialized function instead of panicking, as it's now a valid error.

If that sounds good, I'll start a PR

@NicholasLYang
Copy link

After looking at the code some more, the error handling is actually the tricky bit. Right now fn emit doesn't return a Result, so we can't error cleanly on trying to emit an uninitialized function. We could change that, but it'd be a somewhat larger change.

@NicholasLYang
Copy link

Bump @alexcrichton

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants