From 5231ca1722d15c95b1236b820eb69e9f553eadb9 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 5 Feb 2024 16:06:29 -0800 Subject: [PATCH 01/17] start --- src/passes/StringLowering.cpp | 23 ++++++++++++++++ .../passes/string-lowering-instructions.wast | 27 +++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 test/lit/passes/string-lowering-instructions.wast diff --git a/src/passes/StringLowering.cpp b/src/passes/StringLowering.cpp index 31e41b9e8c7..59a498985e1 100644 --- a/src/passes/StringLowering.cpp +++ b/src/passes/StringLowering.cpp @@ -51,6 +51,7 @@ struct StringGathering : public Pass { processModule(module); addGlobals(module); replaceStrings(module); + replaceInstructions(module); } // Scan the entire wasm to find the relevant strings to populate our global @@ -174,6 +175,28 @@ struct StringGathering : public Pass { *stringPtr = builder.makeGlobalGet(globalName, nnstringref); } } + + void replaceInstructions(Module* module) { + struct Replacer : public WalkerPass> { + bool isFunctionParallel() override { return true; } + + Replacer(NameInfoMap& infos) : infos(infos) {} + + std::unique_ptr create() override { + return std::make_unique(infos); + } + + void visitStringAs(StringAs* curr) { + // There is no difference between strings and views with imported + // strings: they are all just JS strings. + replaceCurrent(curr->ref); + } + }; + + Replacer replacer(infos); + replacer.run(getPassRunner(), module); + replacer.walkModuleCode(module); + } }; struct StringLowering : public StringGathering { diff --git a/test/lit/passes/string-lowering-instructions.wast b/test/lit/passes/string-lowering-instructions.wast new file mode 100644 index 00000000000..be246c33d05 --- /dev/null +++ b/test/lit/passes/string-lowering-instructions.wast @@ -0,0 +1,27 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. + +;; RUN: foreach %s %t wasm-opt --string-lowering -all -S -o - | filecheck %s + +(module + (func $string.as + (param $a stringref) + (param $b stringview_wtf8) + (param $c stringview_wtf16) + (param $d stringview_iter) + (local.set $b ;; validate the output type + (string.as_wtf8 + (local.get $a) + ) + ) + (local.set $c + (string.as_wtf16 + (local.get $a) + ) + ) + (local.set $d + (string.as_iter + (local.get $a) + ) + ) + ) +) From ee1cd026f6642c7656e08f4b3c3abd356c605fc9 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 5 Feb 2024 16:09:36 -0800 Subject: [PATCH 02/17] work --- src/passes/StringLowering.cpp | 53 ++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/src/passes/StringLowering.cpp b/src/passes/StringLowering.cpp index 59a498985e1..3c69edb16cb 100644 --- a/src/passes/StringLowering.cpp +++ b/src/passes/StringLowering.cpp @@ -51,7 +51,6 @@ struct StringGathering : public Pass { processModule(module); addGlobals(module); replaceStrings(module); - replaceInstructions(module); } // Scan the entire wasm to find the relevant strings to populate our global @@ -175,28 +174,6 @@ struct StringGathering : public Pass { *stringPtr = builder.makeGlobalGet(globalName, nnstringref); } } - - void replaceInstructions(Module* module) { - struct Replacer : public WalkerPass> { - bool isFunctionParallel() override { return true; } - - Replacer(NameInfoMap& infos) : infos(infos) {} - - std::unique_ptr create() override { - return std::make_unique(infos); - } - - void visitStringAs(StringAs* curr) { - // There is no difference between strings and views with imported - // strings: they are all just JS strings. - replaceCurrent(curr->ref); - } - }; - - Replacer replacer(infos); - replacer.run(getPassRunner(), module); - replacer.walkModuleCode(module); - } }; struct StringLowering : public StringGathering { @@ -214,6 +191,9 @@ struct StringLowering : public StringGathering { // Remove all HeapType::string etc. in favor of externref. updateTypes(module); + // Replace string.* etc. operations with imported ones. + replaceInstructions(module); + // Disable the feature here after we lowered everything away. module->features.disable(FeatureSet::Strings); } @@ -248,9 +228,36 @@ struct StringLowering : public StringGathering { void updateTypes(Module* module) { TypeMapper::TypeUpdates updates; + // There is no difference between strings and views with imported strings: + // they are all just JS strings, so they all turn into externref. updates[HeapType::string] = HeapType::ext; + updates[HeapType::stringview_wtf8] = HeapType::ext; + updates[HeapType::stringview_wtf16] = HeapType::ext; + updates[HeapType::stringview_iter] = HeapType::ext; TypeMapper(*module, updates).map(); } + + void replaceInstructions(Module* module) { + struct Replacer : public WalkerPass> { + bool isFunctionParallel() override { return true; } + + Replacer(NameInfoMap& infos) : infos(infos) {} + + std::unique_ptr create() override { + return std::make_unique(infos); + } + + void visitStringAs(StringAs* curr) { + // There is no difference between strings and views with imported + // strings: they are all just JS strings, so no conversion is needed. + replaceCurrent(curr->ref); + } + }; + + Replacer replacer(infos); + replacer.run(getPassRunner(), module); + replacer.walkModuleCode(module); + } }; Pass* createStringGatheringPass() { return new StringGathering(); } From 4de58c0536da635578391beb60cf44a6bc4bc0f3 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 5 Feb 2024 16:17:52 -0800 Subject: [PATCH 03/17] work --- src/passes/StringLowering.cpp | 39 ++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/src/passes/StringLowering.cpp b/src/passes/StringLowering.cpp index 3c69edb16cb..a3eaad3587f 100644 --- a/src/passes/StringLowering.cpp +++ b/src/passes/StringLowering.cpp @@ -237,15 +237,16 @@ struct StringLowering : public StringGathering { TypeMapper(*module, updates).map(); } - void replaceInstructions(Module* module) { - struct Replacer : public WalkerPass> { - bool isFunctionParallel() override { return true; } + enum StringImport { + }; - Replacer(NameInfoMap& infos) : infos(infos) {} + void replaceInstructions(Module* module) { + // As we work in parallel we'll collect the set of necessary imports to add. + using NeededImports = std::unordered_set; - std::unique_ptr create() override { - return std::make_unique(infos); - } + // Process functions in parallel. + struct Replacer : public PostWalker { + Replacer(NeededImports& imports) : imports(imports) {} void visitStringAs(StringAs* curr) { // There is no difference between strings and views with imported @@ -254,9 +255,27 @@ struct StringLowering : public StringGathering { } }; - Replacer replacer(infos); - replacer.run(getPassRunner(), module); - replacer.walkModuleCode(module); + ModuleUtils::ParallelFunctionAnalysis analysis( + *module, [&](Function* func, NeededImports& imports) { + if (!func->imported()) { + NewFinder(imports).walk(func->body); + } + }); + + // Also scan global code. + NeededImports allImports; + Replacer(allImports).walkModuleCode(module); + + // Gather all the imports from the functions. + for (auto& [_, imports] : analysis.map) { + for (auto import : imports) { + allImports.insert(import); + } + } + + // Add the imports to the module. + for (StringImport import = StringImport::First; import++; import != StringImport::Last) { + } } }; From c60ae8e92a10c87fc1d662b1ec2e85d665af86be Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 5 Feb 2024 16:18:50 -0800 Subject: [PATCH 04/17] work --- src/passes/StringLowering.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/passes/StringLowering.cpp b/src/passes/StringLowering.cpp index a3eaad3587f..16e0218423b 100644 --- a/src/passes/StringLowering.cpp +++ b/src/passes/StringLowering.cpp @@ -238,6 +238,10 @@ struct StringLowering : public StringGathering { } enum StringImport { + Nothing = 0, + + First = 0, + Last = 1, }; void replaceInstructions(Module* module) { @@ -275,6 +279,9 @@ struct StringLowering : public StringGathering { // Add the imports to the module. for (StringImport import = StringImport::First; import++; import != StringImport::Last) { + if (allImports.count(import)) { + // Add + } } } }; From 5728efa3424d03dbe09ccfbddf5da82b859062ea Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 5 Feb 2024 16:19:44 -0800 Subject: [PATCH 05/17] work --- src/passes/StringLowering.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/passes/StringLowering.cpp b/src/passes/StringLowering.cpp index 16e0218423b..8e63ffce950 100644 --- a/src/passes/StringLowering.cpp +++ b/src/passes/StringLowering.cpp @@ -250,6 +250,8 @@ struct StringLowering : public StringGathering { // Process functions in parallel. struct Replacer : public PostWalker { + NeededImports& imports; + Replacer(NeededImports& imports) : imports(imports) {} void visitStringAs(StringAs* curr) { @@ -262,7 +264,7 @@ struct StringLowering : public StringGathering { ModuleUtils::ParallelFunctionAnalysis analysis( *module, [&](Function* func, NeededImports& imports) { if (!func->imported()) { - NewFinder(imports).walk(func->body); + Replacer(imports).walk(func->body); } }); @@ -278,7 +280,7 @@ struct StringLowering : public StringGathering { } // Add the imports to the module. - for (StringImport import = StringImport::First; import++; import != StringImport::Last) { + for (StringImport import = StringImport::First; import != StringImport::Last; import++) { if (allImports.count(import)) { // Add } From caba9e8fb5bbc7d455d38d275cf30ec56201b078 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 5 Feb 2024 16:21:05 -0800 Subject: [PATCH 06/17] work --- src/passes/StringLowering.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/passes/StringLowering.cpp b/src/passes/StringLowering.cpp index 8e63ffce950..5ec4af7f677 100644 --- a/src/passes/StringLowering.cpp +++ b/src/passes/StringLowering.cpp @@ -280,8 +280,8 @@ struct StringLowering : public StringGathering { } // Add the imports to the module. - for (StringImport import = StringImport::First; import != StringImport::Last; import++) { - if (allImports.count(import)) { + for (int import = StringImport::First; import != StringImport::Last; import++) { + if (allImports.count(StringImport(import))) { // Add } } From 4e38fc4d0b2c2bab40eff786585890e8174d8cd0 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 5 Feb 2024 16:22:00 -0800 Subject: [PATCH 07/17] test --- src/passes/StringLowering.cpp | 4 +++- test/lit/passes/string-lowering-instructions.wast | 13 +++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/passes/StringLowering.cpp b/src/passes/StringLowering.cpp index 5ec4af7f677..be95a7297cd 100644 --- a/src/passes/StringLowering.cpp +++ b/src/passes/StringLowering.cpp @@ -280,7 +280,9 @@ struct StringLowering : public StringGathering { } // Add the imports to the module. - for (int import = StringImport::First; import != StringImport::Last; import++) { + for (int import = StringImport::First; + import != StringImport::Last; + import++) { if (allImports.count(StringImport(import))) { // Add } diff --git a/test/lit/passes/string-lowering-instructions.wast b/test/lit/passes/string-lowering-instructions.wast index be246c33d05..fa1a42d29eb 100644 --- a/test/lit/passes/string-lowering-instructions.wast +++ b/test/lit/passes/string-lowering-instructions.wast @@ -3,6 +3,19 @@ ;; RUN: foreach %s %t wasm-opt --string-lowering -all -S -o - | filecheck %s (module + ;; CHECK: (type $0 (func (param externref externref externref externref))) + + ;; CHECK: (func $string.as (type $0) (param $a externref) (param $b externref) (param $c externref) (param $d externref) + ;; CHECK-NEXT: (local.set $b + ;; CHECK-NEXT: (local.get $a) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.set $c + ;; CHECK-NEXT: (local.get $a) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.set $d + ;; CHECK-NEXT: (local.get $a) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) (func $string.as (param $a stringref) (param $b stringview_wtf8) From 0fdf5a41843ae21fe0da910f3eb13ea952a6abaf Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 5 Feb 2024 16:25:35 -0800 Subject: [PATCH 08/17] test --- src/passes/StringLowering.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/passes/StringLowering.cpp b/src/passes/StringLowering.cpp index be95a7297cd..1a86fa179c5 100644 --- a/src/passes/StringLowering.cpp +++ b/src/passes/StringLowering.cpp @@ -247,6 +247,7 @@ struct StringLowering : public StringGathering { void replaceInstructions(Module* module) { // As we work in parallel we'll collect the set of necessary imports to add. using NeededImports = std::unordered_set; + // XXX aybe the opposite: create all imports eagerly, let optimizer remov // Process functions in parallel. struct Replacer : public PostWalker { @@ -254,6 +255,19 @@ struct StringLowering : public StringGathering { Replacer(NeededImports& imports) : imports(imports) {} + void visitStringNew(StringNew* curr) { + switch (curr->op) { + case StringNewWTF16Array: + printMedium(o, "string.new_wtf16_array"); + break; + case StringNewFromCodePoint: + printMedium(o, "string.from_code_point"); + break; + default: + WASM_UNREACHABLE("TODO: all of string.new*"); + } + } + void visitStringAs(StringAs* curr) { // There is no difference between strings and views with imported // strings: they are all just JS strings, so no conversion is needed. From d18fb2308ab371b088fc835433c0e39db1db058e Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 6 Feb 2024 13:01:14 -0800 Subject: [PATCH 09/17] work --- src/passes/StringLowering.cpp | 81 +++++++++++++++++------------------ 1 file changed, 39 insertions(+), 42 deletions(-) diff --git a/src/passes/StringLowering.cpp b/src/passes/StringLowering.cpp index 1a86fa179c5..33f2707a0ff 100644 --- a/src/passes/StringLowering.cpp +++ b/src/passes/StringLowering.cpp @@ -237,32 +237,52 @@ struct StringLowering : public StringGathering { TypeMapper(*module, updates).map(); } - enum StringImport { - Nothing = 0, + // Imported string functions. + Name fromCharCodeArrayImport; + Name fromCodePointImport; - First = 0, - Last = 1, - }; + auto nnExt = Type(HeapType::ext, NonNullable); void replaceInstructions(Module* module) { - // As we work in parallel we'll collect the set of necessary imports to add. - using NeededImports = std::unordered_set; - // XXX aybe the opposite: create all imports eagerly, let optimizer remov + // Add all the possible imports up front, to avoid adding them during + // parallel work. Optimizations can remove unneeded ones later. + Builder builder(*module); + auto array16 = Array(Field(Field::i16, Mutable)); - // Process functions in parallel. - struct Replacer : public PostWalker { - NeededImports& imports; + { + fromCharCodeArrayImport = Names::getValidFunctionName( + *module, "string.fromCharCodeArray"); + auto sig = Signature({Type(array16, Nullable), Type::i32, Type::i32}, nnExt); + module->addFunction(builder.makeFunction(fromCharCodeArrayImport, sig, {})); + } + { + fromCodePointImport = Names::getValidFunctionName( + *module, "string.fromCodePoint"); + auto sig = Signature({Type::i32, nnExt); + module->addFunction(builder.makeFunction(fromCodePointImport, sig, {})); + } - Replacer(NeededImports& imports) : imports(imports) {} + // Replace the string instructions in parallel. + struct Replacer : public WalkerPass> { + bool isFunctionParallel() override { return true; } + + StringLowering& lowering; + + std::unique_ptr create() override { + return std::make_unique(lowering); + } + + Replacer(StringLowering& lowering) : lowering(lowering) {} void visitStringNew(StringNew* curr) { + Builder builder(*getModule()); switch (curr->op) { case StringNewWTF16Array: - printMedium(o, "string.new_wtf16_array"); - break; + replaceCurrent(builder.makeCall(lowering.fromCharCodeArrayImport, {curr->ptr, curr->start, curr->end}, lowering.nnExt)) + return; case StringNewFromCodePoint: - printMedium(o, "string.from_code_point"); - break; + replaceCurrent(builder.makeCall(lowering.fromCodePointImport, {curr->ptr}, lowering.nnExt)) + return; default: WASM_UNREACHABLE("TODO: all of string.new*"); } @@ -275,32 +295,9 @@ struct StringLowering : public StringGathering { } }; - ModuleUtils::ParallelFunctionAnalysis analysis( - *module, [&](Function* func, NeededImports& imports) { - if (!func->imported()) { - Replacer(imports).walk(func->body); - } - }); - - // Also scan global code. - NeededImports allImports; - Replacer(allImports).walkModuleCode(module); - - // Gather all the imports from the functions. - for (auto& [_, imports] : analysis.map) { - for (auto import : imports) { - allImports.insert(import); - } - } - - // Add the imports to the module. - for (int import = StringImport::First; - import != StringImport::Last; - import++) { - if (allImports.count(StringImport(import))) { - // Add - } - } + Replacer replacer(*this); + replacer.run(getPassRunner(), module); + replacer.walkModuleCode(module); } }; From eedd15c29dc9528304ab43a039aca14d84d7d300 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 6 Feb 2024 13:02:06 -0800 Subject: [PATCH 10/17] work --- src/passes/StringLowering.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/passes/StringLowering.cpp b/src/passes/StringLowering.cpp index 33f2707a0ff..f6f4483f29c 100644 --- a/src/passes/StringLowering.cpp +++ b/src/passes/StringLowering.cpp @@ -241,7 +241,7 @@ struct StringLowering : public StringGathering { Name fromCharCodeArrayImport; Name fromCodePointImport; - auto nnExt = Type(HeapType::ext, NonNullable); + Type nnExt = Type(HeapType::ext, NonNullable); void replaceInstructions(Module* module) { // Add all the possible imports up front, to avoid adding them during @@ -258,7 +258,7 @@ struct StringLowering : public StringGathering { { fromCodePointImport = Names::getValidFunctionName( *module, "string.fromCodePoint"); - auto sig = Signature({Type::i32, nnExt); + auto sig = Signature(Type::i32, nnExt); module->addFunction(builder.makeFunction(fromCodePointImport, sig, {})); } @@ -278,10 +278,10 @@ struct StringLowering : public StringGathering { Builder builder(*getModule()); switch (curr->op) { case StringNewWTF16Array: - replaceCurrent(builder.makeCall(lowering.fromCharCodeArrayImport, {curr->ptr, curr->start, curr->end}, lowering.nnExt)) + replaceCurrent(builder.makeCall(lowering.fromCharCodeArrayImport, {curr->ptr, curr->start, curr->end}, lowering.nnExt)); return; case StringNewFromCodePoint: - replaceCurrent(builder.makeCall(lowering.fromCodePointImport, {curr->ptr}, lowering.nnExt)) + replaceCurrent(builder.makeCall(lowering.fromCodePointImport, {curr->ptr}, lowering.nnExt)); return; default: WASM_UNREACHABLE("TODO: all of string.new*"); From f377e3b393f7b6c1fd7d7874e90a6b132485eccc Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 6 Feb 2024 13:11:35 -0800 Subject: [PATCH 11/17] start --- src/passes/StringLowering.cpp | 31 ++++++++++--------- test/lit/passes/string-gathering.wast | 20 ++++++++++++ .../passes/string-lowering-instructions.wast | 10 ++++++ 3 files changed, 47 insertions(+), 14 deletions(-) diff --git a/src/passes/StringLowering.cpp b/src/passes/StringLowering.cpp index f6f4483f29c..bae6fb1889c 100644 --- a/src/passes/StringLowering.cpp +++ b/src/passes/StringLowering.cpp @@ -243,24 +243,27 @@ struct StringLowering : public StringGathering { Type nnExt = Type(HeapType::ext, NonNullable); + // Creates an imported string function, returning its name (which is equal to + // the true name of the import, if there is no conflict). + Name addImport(Module* module, Name trueName, Type params, Type results) { + auto name = Names::getValidFunctionName(*module, trueName); + auto sig = Signature(params, results); + Builder builder(*module); + auto* func = module->addFunction(builder.makeFunction(name, sig, {})); + func->module = "wasm:js-string"; + func->base = trueName; + return name; + } + void replaceInstructions(Module* module) { // Add all the possible imports up front, to avoid adding them during // parallel work. Optimizations can remove unneeded ones later. - Builder builder(*module); - auto array16 = Array(Field(Field::i16, Mutable)); + auto nullArray16 = Type(Array(Field(Field::i16, Mutable)), Nullable); - { - fromCharCodeArrayImport = Names::getValidFunctionName( - *module, "string.fromCharCodeArray"); - auto sig = Signature({Type(array16, Nullable), Type::i32, Type::i32}, nnExt); - module->addFunction(builder.makeFunction(fromCharCodeArrayImport, sig, {})); - } - { - fromCodePointImport = Names::getValidFunctionName( - *module, "string.fromCodePoint"); - auto sig = Signature(Type::i32, nnExt); - module->addFunction(builder.makeFunction(fromCodePointImport, sig, {})); - } + // string.fromCharCodeArray: array, start, end -> ext + fromCharCodeArrayImport = addImport(module, "fromCharCodeArray", {nullArray16, Type::i32, Type::i32}, nnExt); + // string.fromCodePoint: codepoint -> ext + fromCodePointImport = addImport(module, "fromCodePoint", Type::i32, nnExt); // Replace the string instructions in parallel. struct Replacer : public WalkerPass> { diff --git a/test/lit/passes/string-gathering.wast b/test/lit/passes/string-gathering.wast index 657858fc050..8c315ddc140 100644 --- a/test/lit/passes/string-gathering.wast +++ b/test/lit/passes/string-gathering.wast @@ -27,12 +27,22 @@ ;; CHECK: (global $global2 stringref (global.get $string.const_bar)) ;; LOWER: (type $0 (func)) + ;; LOWER: (type $1 (array (mut i16))) + + ;; LOWER: (type $2 (func (param (ref null $1) i32 i32) (result (ref extern)))) + + ;; LOWER: (type $3 (func (param i32) (result (ref extern)))) + ;; LOWER: (import "string.const" "0" (global $string.const_bar (ref extern))) ;; LOWER: (import "string.const" "1" (global $string.const_other (ref extern))) ;; LOWER: (import "string.const" "2" (global $global (ref extern))) + ;; LOWER: (import "wasm:js-string" "fromCharCodeArray" (func $fromCharCodeArray (type $2) (param (ref null $1) i32 i32) (result (ref extern)))) + + ;; LOWER: (import "wasm:js-string" "fromCodePoint" (func $fromCodePoint (type $3) (param i32) (result (ref extern)))) + ;; LOWER: (global $global2 externref (global.get $string.const_bar)) (global $global2 (ref null string) (string.const "bar")) @@ -111,6 +121,12 @@ ;; Multiple possible reusable globals. Also test ignoring of imports. (module ;; CHECK: (import "a" "b" (global $import (ref string))) + ;; LOWER: (type $0 (array (mut i16))) + + ;; LOWER: (type $1 (func (param (ref null $0) i32 i32) (result (ref extern)))) + + ;; LOWER: (type $2 (func (param i32) (result (ref extern)))) + ;; LOWER: (import "a" "b" (global $import (ref extern))) (import "a" "b" (global $import (ref string))) @@ -122,6 +138,10 @@ ;; LOWER: (import "string.const" "1" (global $global4 (ref extern))) + ;; LOWER: (import "wasm:js-string" "fromCharCodeArray" (func $fromCharCodeArray (type $1) (param (ref null $0) i32 i32) (result (ref extern)))) + + ;; LOWER: (import "wasm:js-string" "fromCodePoint" (func $fromCodePoint (type $2) (param i32) (result (ref extern)))) + ;; LOWER: (global $global2 (ref extern) (global.get $global1)) (global $global2 (ref string) (string.const "foo")) diff --git a/test/lit/passes/string-lowering-instructions.wast b/test/lit/passes/string-lowering-instructions.wast index fa1a42d29eb..3a8e620a5e4 100644 --- a/test/lit/passes/string-lowering-instructions.wast +++ b/test/lit/passes/string-lowering-instructions.wast @@ -5,6 +5,16 @@ (module ;; CHECK: (type $0 (func (param externref externref externref externref))) + ;; CHECK: (type $1 (array (mut i16))) + + ;; CHECK: (type $2 (func (param (ref null $1) i32 i32) (result (ref extern)))) + + ;; CHECK: (type $3 (func (param i32) (result (ref extern)))) + + ;; CHECK: (import "wasm:js-string" "fromCharCodeArray" (func $fromCharCodeArray (type $2) (param (ref null $1) i32 i32) (result (ref extern)))) + + ;; CHECK: (import "wasm:js-string" "fromCodePoint" (func $fromCodePoint (type $3) (param i32) (result (ref extern)))) + ;; CHECK: (func $string.as (type $0) (param $a externref) (param $b externref) (param $c externref) (param $d externref) ;; CHECK-NEXT: (local.set $b ;; CHECK-NEXT: (local.get $a) From 83f61f4f63bbcb3a500f71af88cbbf88db037e2a Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 6 Feb 2024 13:23:21 -0800 Subject: [PATCH 12/17] test --- .../passes/string-lowering-instructions.wast | 58 ++++++++++++++++--- 1 file changed, 50 insertions(+), 8 deletions(-) diff --git a/test/lit/passes/string-lowering-instructions.wast b/test/lit/passes/string-lowering-instructions.wast index 3a8e620a5e4..dfa3fde568c 100644 --- a/test/lit/passes/string-lowering-instructions.wast +++ b/test/lit/passes/string-lowering-instructions.wast @@ -3,19 +3,25 @@ ;; RUN: foreach %s %t wasm-opt --string-lowering -all -S -o - | filecheck %s (module - ;; CHECK: (type $0 (func (param externref externref externref externref))) + ;; CHECK: (type $array16 (array (mut i16))) + (type $array16 (array (mut i16))) - ;; CHECK: (type $1 (array (mut i16))) + ;; CHECK: (rec + ;; CHECK-NEXT: (type $1 (func)) - ;; CHECK: (type $2 (func (param (ref null $1) i32 i32) (result (ref extern)))) + ;; CHECK: (type $2 (func (param (ref $array16)))) - ;; CHECK: (type $3 (func (param i32) (result (ref extern)))) + ;; CHECK: (type $3 (func (param externref externref externref externref))) - ;; CHECK: (import "wasm:js-string" "fromCharCodeArray" (func $fromCharCodeArray (type $2) (param (ref null $1) i32 i32) (result (ref extern)))) + ;; CHECK: (type $4 (func (param (ref null $array16) i32 i32) (result (ref extern)))) - ;; CHECK: (import "wasm:js-string" "fromCodePoint" (func $fromCodePoint (type $3) (param i32) (result (ref extern)))) + ;; CHECK: (type $5 (func (param i32) (result (ref extern)))) - ;; CHECK: (func $string.as (type $0) (param $a externref) (param $b externref) (param $c externref) (param $d externref) + ;; CHECK: (import "wasm:js-string" "fromCharCodeArray" (func $fromCharCodeArray (type $4) (param (ref null $array16) i32 i32) (result (ref extern)))) + + ;; CHECK: (import "wasm:js-string" "fromCodePoint" (func $fromCodePoint (type $5) (param i32) (result (ref extern)))) + + ;; CHECK: (func $string.as (type $3) (param $a externref) (param $b externref) (param $c externref) (param $d externref) ;; CHECK-NEXT: (local.set $b ;; CHECK-NEXT: (local.get $a) ;; CHECK-NEXT: ) @@ -31,7 +37,9 @@ (param $b stringview_wtf8) (param $c stringview_wtf16) (param $d stringview_iter) - (local.set $b ;; validate the output type + ;; These operations all vanish in the lowering, as they all become extref + ;; (JS strings). + (local.set $b (string.as_wtf8 (local.get $a) ) @@ -47,4 +55,38 @@ ) ) ) + + ;; CHECK: (func $string.new.gc (type $2) (param $array16 (ref $array16)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (call $fromCharCodeArray + ;; CHECK-NEXT: (local.get $array16) + ;; CHECK-NEXT: (i32.const 7) + ;; CHECK-NEXT: (i32.const 8) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $string.new.gc (param $array16 (ref $array16)) + (drop + (string.new_wtf16_array + (local.get $array16) + (i32.const 7) + (i32.const 8) + ) + ) + ) + + ;; CHECK: (func $string.from_code_point (type $1) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (call $fromCodePoint + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $string.from_code_point + (drop + (string.from_code_point + (i32.const 1) + ) + ) + ) ) From dbb73cc76a616b3b15daa1fa1be2265f11cadc69 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 6 Feb 2024 13:23:36 -0800 Subject: [PATCH 13/17] format --- src/passes/StringLowering.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/passes/StringLowering.cpp b/src/passes/StringLowering.cpp index bae6fb1889c..56c8e7285c7 100644 --- a/src/passes/StringLowering.cpp +++ b/src/passes/StringLowering.cpp @@ -261,7 +261,8 @@ struct StringLowering : public StringGathering { auto nullArray16 = Type(Array(Field(Field::i16, Mutable)), Nullable); // string.fromCharCodeArray: array, start, end -> ext - fromCharCodeArrayImport = addImport(module, "fromCharCodeArray", {nullArray16, Type::i32, Type::i32}, nnExt); + fromCharCodeArrayImport = addImport( + module, "fromCharCodeArray", {nullArray16, Type::i32, Type::i32}, nnExt); // string.fromCodePoint: codepoint -> ext fromCodePointImport = addImport(module, "fromCodePoint", Type::i32, nnExt); @@ -281,10 +282,13 @@ struct StringLowering : public StringGathering { Builder builder(*getModule()); switch (curr->op) { case StringNewWTF16Array: - replaceCurrent(builder.makeCall(lowering.fromCharCodeArrayImport, {curr->ptr, curr->start, curr->end}, lowering.nnExt)); + replaceCurrent(builder.makeCall(lowering.fromCharCodeArrayImport, + {curr->ptr, curr->start, curr->end}, + lowering.nnExt)); return; case StringNewFromCodePoint: - replaceCurrent(builder.makeCall(lowering.fromCodePointImport, {curr->ptr}, lowering.nnExt)); + replaceCurrent(builder.makeCall( + lowering.fromCodePointImport, {curr->ptr}, lowering.nnExt)); return; default: WASM_UNREACHABLE("TODO: all of string.new*"); From 8d9ae93ee4f62ca718690c1dd1ed238506cef2a6 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 6 Feb 2024 13:25:11 -0800 Subject: [PATCH 14/17] fix --- .../passes/string-lowering-instructions.wast | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/test/lit/passes/string-lowering-instructions.wast b/test/lit/passes/string-lowering-instructions.wast index dfa3fde568c..043f60dfebc 100644 --- a/test/lit/passes/string-lowering-instructions.wast +++ b/test/lit/passes/string-lowering-instructions.wast @@ -3,13 +3,15 @@ ;; RUN: foreach %s %t wasm-opt --string-lowering -all -S -o - | filecheck %s (module + ;; CHECK: (type $0 (func)) + ;; CHECK: (type $array16 (array (mut i16))) (type $array16 (array (mut i16))) - ;; CHECK: (rec - ;; CHECK-NEXT: (type $1 (func)) - ;; CHECK: (type $2 (func (param (ref $array16)))) + ;; An existing import with a colliding name, to check that we handle that. + ;; CHECK: (rec + ;; CHECK-NEXT: (type $2 (func (param (ref $array16)))) ;; CHECK: (type $3 (func (param externref externref externref externref))) @@ -17,9 +19,12 @@ ;; CHECK: (type $5 (func (param i32) (result (ref extern)))) + ;; CHECK: (import "existing" "import" (func $fromCodePoint (type $0))) + (import "existing" "import" (func $fromCodePoint)) + ;; CHECK: (import "wasm:js-string" "fromCharCodeArray" (func $fromCharCodeArray (type $4) (param (ref null $array16) i32 i32) (result (ref extern)))) - ;; CHECK: (import "wasm:js-string" "fromCodePoint" (func $fromCodePoint (type $5) (param i32) (result (ref extern)))) + ;; CHECK: (import "wasm:js-string" "fromCodePoint" (func $fromCodePoint_5 (type $5) (param i32) (result (ref extern)))) ;; CHECK: (func $string.as (type $3) (param $a externref) (param $b externref) (param $c externref) (param $d externref) ;; CHECK-NEXT: (local.set $b @@ -75,9 +80,9 @@ ) ) - ;; CHECK: (func $string.from_code_point (type $1) + ;; CHECK: (func $string.from_code_point (type $0) ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (call $fromCodePoint + ;; CHECK-NEXT: (call $fromCodePoint_5 ;; CHECK-NEXT: (i32.const 1) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) From d2596416017afcf69d7c02c083fc90e873661855 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 6 Feb 2024 13:27:47 -0800 Subject: [PATCH 15/17] fix --- src/passes/StringLowering.cpp | 8 ++++++-- test/lit/passes/string-lowering-instructions.wast | 6 ++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/passes/StringLowering.cpp b/src/passes/StringLowering.cpp index 56c8e7285c7..b268d18a7c2 100644 --- a/src/passes/StringLowering.cpp +++ b/src/passes/StringLowering.cpp @@ -241,6 +241,11 @@ struct StringLowering : public StringGathering { Name fromCharCodeArrayImport; Name fromCodePointImport; + // The name of the module to import string functions from. + Name WasmStringsModule = "wasm:js-string"; + + // Common types used in imports. + auto nullArray16 = Type(Array(Field(Field::i16, Mutable)), Nullable); Type nnExt = Type(HeapType::ext, NonNullable); // Creates an imported string function, returning its name (which is equal to @@ -250,7 +255,7 @@ struct StringLowering : public StringGathering { auto sig = Signature(params, results); Builder builder(*module); auto* func = module->addFunction(builder.makeFunction(name, sig, {})); - func->module = "wasm:js-string"; + func->module = WasmStringsModule; func->base = trueName; return name; } @@ -258,7 +263,6 @@ struct StringLowering : public StringGathering { void replaceInstructions(Module* module) { // Add all the possible imports up front, to avoid adding them during // parallel work. Optimizations can remove unneeded ones later. - auto nullArray16 = Type(Array(Field(Field::i16, Mutable)), Nullable); // string.fromCharCodeArray: array, start, end -> ext fromCharCodeArrayImport = addImport( diff --git a/test/lit/passes/string-lowering-instructions.wast b/test/lit/passes/string-lowering-instructions.wast index 043f60dfebc..05d555ef0df 100644 --- a/test/lit/passes/string-lowering-instructions.wast +++ b/test/lit/passes/string-lowering-instructions.wast @@ -8,8 +8,6 @@ ;; CHECK: (type $array16 (array (mut i16))) (type $array16 (array (mut i16))) - - ;; An existing import with a colliding name, to check that we handle that. ;; CHECK: (rec ;; CHECK-NEXT: (type $2 (func (param (ref $array16)))) @@ -19,8 +17,8 @@ ;; CHECK: (type $5 (func (param i32) (result (ref extern)))) - ;; CHECK: (import "existing" "import" (func $fromCodePoint (type $0))) - (import "existing" "import" (func $fromCodePoint)) + ;; CHECK: (import "colliding" "name" (func $fromCodePoint (type $0))) + (import "colliding" "name" (func $fromCodePoint)) ;; CHECK: (import "wasm:js-string" "fromCharCodeArray" (func $fromCharCodeArray (type $4) (param (ref null $array16) i32 i32) (result (ref extern)))) From 402da59dd9c13079cebfc5182e6fea1ee514d3b8 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 6 Feb 2024 13:45:18 -0800 Subject: [PATCH 16/17] error --- src/passes/StringLowering.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/passes/StringLowering.cpp b/src/passes/StringLowering.cpp index b268d18a7c2..e4b3ed865d5 100644 --- a/src/passes/StringLowering.cpp +++ b/src/passes/StringLowering.cpp @@ -245,7 +245,7 @@ struct StringLowering : public StringGathering { Name WasmStringsModule = "wasm:js-string"; // Common types used in imports. - auto nullArray16 = Type(Array(Field(Field::i16, Mutable)), Nullable); + Type nullArray16 = Type(Array(Field(Field::i16, Mutable)), Nullable); Type nnExt = Type(HeapType::ext, NonNullable); // Creates an imported string function, returning its name (which is equal to From aba03bd5856a080cefcf37afa873c89c6462660d Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 6 Feb 2024 14:11:47 -0800 Subject: [PATCH 17/17] avoid fuzzer issues --- scripts/fuzz_opt.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/fuzz_opt.py b/scripts/fuzz_opt.py index f839984c255..251b23d033a 100755 --- a/scripts/fuzz_opt.py +++ b/scripts/fuzz_opt.py @@ -286,6 +286,7 @@ def is_git_repo(): # TODO: fuzzer and interpreter support for strings 'strings.wast', 'simplify-locals-strings.wast', + 'string-lowering-instructions.wast', # TODO: fuzzer and interpreter support for extern conversions 'extern-conversions.wast', # ignore DWARF because it is incompatible with multivalue atm