Repository navigation
[Strings] Handle fromCharCode and cast imports in StringLifting/Lowering - #9373
jeremy-code wants to merge 7 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support and optimizations for fromCharCode and cast operations in both the StringLifting and StringLowering passes. In StringLifting, fromCharCode is mapped to fromCodePoint with a 0xFFFF mask, and cast is lifted to a ref.cast to string. In StringLowering, casts to string are lowered to imported cast calls, and fromCodePoint with a 0xFFFF mask is optimized back to fromCharCode. The feedback suggests optimizing the lowering of nullable casts by skipping the redundant null check when the input operand is already known to be non-nullable.
StringLifting: fromCharCode(x) becomes string.from_code_point(x & 0xFFFF) and cast becomes non-nullable ref.cast to string. StringLowering: string.from_code_point(x & 0xFFFF) or equivalent becomes fromCharCode(x) and non-nullable ref.cast to string becomes cast(). Nullable ref.cast to string checks ref.is_null beforehand.
…-instructions.wast Co-authored-by: Thomas Lively <tlively123@gmail.com>
c26b4b0 to
e55e999
Compare
…st behavior Add a StringNewFromCharCode IR operation. The equivalent stringref operation is string.from_code_point where the codePoint is masked (x & 0xFFFF). Allow ref.cast with strings when strings are enabled, otherwise GC is required. StringLifting: Refinalize IR whenever a call has been replaced (i.e. externref -> stringref). StringLowering: Move replaceInstructions before updateTypes and add visitRefCast in Replacer.
| bool hasStringCasts = | ||
| curr->type.isRef() && | ||
| curr->type.getHeapType().isMaybeShared(HeapType::string); | ||
| shouldBeTrue(getModule()->features.hasGC() || | ||
| (hasStringCasts && getModule()->features.hasStrings()), |
There was a problem hiding this comment.
| bool hasStringCasts = | |
| curr->type.isRef() && | |
| curr->type.getHeapType().isMaybeShared(HeapType::string); | |
| shouldBeTrue(getModule()->features.hasGC() || | |
| (hasStringCasts && getModule()->features.hasStrings()), | |
| bool isStringCast = curr->type.isString(); | |
| shouldBeTrue(getModule()->features.hasGC() || | |
| (isStringCast && getModule()->features.hasStrings()), |
I wonder if we will need to allow casts to nullexternref without GC enabled as well. Can you run the fuzzer (scripts/monitor_fuzz.py) to find out?
Please also add validation tests for this new special case.
There was a problem hiding this comment.
I think the fuzzer is set to disable strings whenever GC is disabled:
Line 2922 in 0c9067a
|
|
||
| ;; CHECK: (import "wasm:js-string" "substring" (func $substring (type $11) (param externref i32 i32) (result (ref extern)))) | ||
|
|
||
| ;; CHECK: (import "wasm:js-string" "fromCharCode" (func $fromCharCode_14 (type $0) (param i32) (result (ref extern)))) |
There was a problem hiding this comment.
I guess it's probably out of scope for this PR, but it seems bad that we end up with duplicate imports after the round trip.
Co-authored-by: Thomas Lively <tlively123@gmail.com>
Co-authored-by: Thomas Lively <tlively123@gmail.com>
…ble or string bottom type to be cast without GC StringFromCharCode now has opcode 0xab. Support unreachable casts and casts to the string bottom type (noextern) without requiring GC.
tlively
left a comment
tlively
left a comment
There was a problem hiding this comment.
LGTM with that addition to the README. Thanks!
StringLifting: fromCharCode(x) becomes string.from_code_point(x & 0xFFFF) and cast becomes non-nullable ref.cast to string.
StringLowering: string.from_code_point(x & 0xFFFF) or equivalent becomes fromCharCode(x) and non-nullable ref.cast to string becomes cast(). Nullable ref.cast to string checks ref.is_null beforehand.