StringLowering #6271
StringLowering #6271
Conversation
Co-authored-by: Thomas Lively <tlively123@gmail.com>
Co-authored-by: Thomas Lively <tlively123@gmail.com>
|
Would you be open to treating this custom section format as a POC? There are some changes that I think we should consider for the productionized version, but that shouldn't have to block experimentation. |
| importIndex++; | ||
| global->init = nullptr; | ||
|
|
||
| auto str = json::Value::make(std::string(c->string.str).c_str()); |
There was a problem hiding this comment.
We should use string_view rather than c_str to properly handle nul bytes. Maybe we can fix the json API in a separate PR?
There was a problem hiding this comment.
Yeah, that's worth fixing separately, I agree.
|
|
||
| void updateTypes(Module* module) { | ||
| TypeMapper::TypeUpdates updates; | ||
| updates[HeapType::string] = HeapType::ext; |
There was a problem hiding this comment.
If a module ever uses the fact that string <: any, this is going to cause problems, but that's probably ok for experimentation.
There was a problem hiding this comment.
Good point. If that is ever an issue it seems like we'd need to internalize/externalize in a lot of places...?
There was a problem hiding this comment.
This is why I think we should make our internal string type be a subtype of extern instead of any, but of course that would break compatibility with the stringref proposal. This is a change we should make once we don't need to support stringref directly anymore and can get away with only supporting imported strings. Hopefully that will be soon?
There was a problem hiding this comment.
I see, yeah, that could help. But implementing a not-quite-stringref has downsides too. I'm not sure what's best in the long term, but we don't need to decide now.
Definitely, yeah, this is just a first version to start iterating. The format here is iirc the best idea @jakobkummerow had in the past, and I can't think of anything better - the section is just JSON that JS can quickly parse into an array of strings and then provide as a single import to wasm. But as we experiment we can explore more. |
|
@gkdn does this look good as a first step? Also please let me know which string instructions you use, that I should implement lowering for (the full stringref proposal is large, but I'm sure we need only a small part of it). |
|
Seems reasonable. I won't be able to test it out until we have the instruction lowering in place since types are converted to externref but can try it when we have other parts in place. The instructions we are currently using: |
|
Sounds good, thanks @gkdn , landing. |
All those in the list from #6271 (comment)
This extends StringGathering by replacing the gathered string globals to imported globals. It adds a custom section with the strings that the imports are expected to provide. It also replaces the string type with extern. This is a complete lowering of strings, except for string operations that are a TODO. After running this, no strings remain in the wasm, and the outside JS is expected to provide the proper imports, which it can do by processing the JSON of the strings in the custom section "string.consts", which looks like ["foo", "bar", ..] That is, an array of strings, which are imported as (import "string.const" "0" (global $string.const_foo (ref extern))) ;; foo (import "string.const" "1" (global $string.const_bar (ref extern))) ;; bar
…ssembly#6283) All those in the list from WebAssembly#6271 (comment)
This extends StringGathering by replacing the gathered string globals to imported
globals. It adds a custom section with the strings that the imports are expected to
provide. It also replaces the string type with extern.
This is a complete lowering of strings, except for string operations that are a TODO.
After running this, no strings remain in the wasm, and the outside JS is expected
to provide the proper imports, which it can do by processing the JSON of the
strings in the custom section "
string.consts", which looks likeThat is, an array of strings, which are imported as
cc @gkdn This is the pass that you will want to run manually at the end of the
pipeline, to get imported strings.