From 206edf6e57bddeaa126a8f2c42e2ab73d5f56702 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Vouillon?= Date: Tue, 27 Feb 2024 10:54:10 -0500 Subject: [PATCH 1/7] Add sourcemap options to wasm-metadce --- src/tools/wasm-metadce.cpp | 33 ++++++++++++++++++++++++++++++++- test/lit/help/wasm-metadce.test | 7 +++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/tools/wasm-metadce.cpp b/src/tools/wasm-metadce.cpp index 5be60c2986d..5acd9832c8c 100644 --- a/src/tools/wasm-metadce.cpp +++ b/src/tools/wasm-metadce.cpp @@ -365,6 +365,9 @@ int main(int argc, const char* argv[]) { bool debugInfo = false; std::string graphFile; bool dump = false; + std::string inputSourceMapFilename; + std::string outputSourceMapFilename; + std::string outputSourceMapUrl; const std::string WasmMetaDCEOption = "wasm-opt options"; @@ -423,6 +426,30 @@ int main(int argc, const char* argv[]) { o->extra["output"] = argument; Colors::setEnabled(false); }) + .add("--input-source-map", + "-ism", + "Consume source map from the specified file", + WasmMetaDCEOption, + Options::Arguments::One, + [&inputSourceMapFilename](Options* o, const std::string& argument) { + inputSourceMapFilename = argument; + }) + .add("--output-source-map", + "-osm", + "Emit source map to the specified file", + WasmMetaDCEOption, + Options::Arguments::One, + [&outputSourceMapFilename](Options* o, const std::string& argument) { + outputSourceMapFilename = argument; + }) + .add("--output-source-map-url", + "-osu", + "Emit specified string as source map URL", + WasmMetaDCEOption, + Options::Arguments::One, + [&outputSourceMapUrl](Options* o, const std::string& argument) { + outputSourceMapUrl = argument; + }) .add("--emit-text", "-S", "Emit text instead of binary for the output file", @@ -470,7 +497,7 @@ int main(int argc, const char* argv[]) { ModuleReader reader; reader.setDWARF(debugInfo); try { - reader.read(options.extra["infile"], wasm); + reader.read(options.extra["infile"], wasm, input); } catch (ParseException& p) { p.dump(std::cerr); Fatal() << "error in parsing wasm input"; @@ -578,6 +605,10 @@ int main(int argc, const char* argv[]) { ModuleWriter writer; writer.setBinary(emitBinary); writer.setDebugInfo(debugInfo); + if (outputSourceMapFilename.size()) { + writer.setSourceMapFilename(outputSourceMapFilename); + writer.setSourceMapUrl(outputSourceMapUrl); + } writer.write(wasm, options.extra["output"]); } diff --git a/test/lit/help/wasm-metadce.test b/test/lit/help/wasm-metadce.test index 4334dbd0166..4d706dc0f1d 100644 --- a/test/lit/help/wasm-metadce.test +++ b/test/lit/help/wasm-metadce.test @@ -53,6 +53,13 @@ ;; CHECK-NEXT: ;; CHECK-NEXT: --output,-o Output file (stdout if not specified) ;; CHECK-NEXT: +;; CHECK-NEXT: --input-source-map,-ism Consume source map from the specified +;; CHECK-NEXT: file +;; CHECK-NEXT: +;; CHECK-NEXT: --output-source-map,-osm Emit source map to the specified file +;; CHECK-NEXT: +;; CHECK-NEXT: --output-source-map-url,-osu Emit specified string as source map URL +;; CHECK-NEXT: ;; CHECK-NEXT: --emit-text,-S Emit text instead of binary for the ;; CHECK-NEXT: output file ;; CHECK-NEXT: From 18b2d635c9825af0629cea51b8afe63a403d9cf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Vouillon?= Date: Tue, 27 Feb 2024 14:20:08 -0500 Subject: [PATCH 2/7] Add sourcemap options to wasm-merge --- src/tools/wasm-merge.cpp | 49 +++++++++++++++++++++++++++++++++-- test/lit/help/wasm-merge.test | 12 +++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/tools/wasm-merge.cpp b/src/tools/wasm-merge.cpp index 0884350cb5b..ea7584f0aa8 100644 --- a/src/tools/wasm-merge.cpp +++ b/src/tools/wasm-merge.cpp @@ -452,6 +452,9 @@ int main(int argc, const char* argv[]) { std::vector inputFileNames; bool emitBinary = true; bool debugInfo = false; + std::map inputSourceMapFilenames; + std::string outputSourceMapFilename; + std::string outputSourceMapUrl; const std::string WasmMergeOption = "wasm-merge options"; @@ -464,7 +467,11 @@ For example, will read foo.wasm and bar.wasm, with names 'foo' and 'bar' respectively, so if the second imports from 'foo', we will see that as an import from the first module after the merge. The merged output will be written to merged.wasm. -Note that filenames and modules names are interleaved (which is hopefully less confusing).)"); +Note that filenames and modules names are interleaved (which is hopefully less confusing). + +Input source maps can be specified by adding an --ism option right after the module name: + + wasm-merge foo.wasm foo --ism foo.wasm.map ...)"); options .add("--output", @@ -485,6 +492,37 @@ Note that filenames and modules names are interleaved (which is hopefully less c inputFileNames.push_back(argument); } }) + .add("--input-source-map", + "-ism", + "Consume source maps from the specified files", + WasmMergeOption, + Options::Arguments::N, + [&](Options* o, const std::string& argument) { + size_t pos = inputFiles.size(); + if (pos == 0 || pos != inputFileNames.size() || + inputSourceMapFilenames.count(pos - 1)) { + std::cerr << "Option '-ism " << argument + << "' should be right after the module name\n"; + exit(EXIT_FAILURE); + } + inputSourceMapFilenames.insert({pos - 1, argument}); + }) + .add("--output-source-map", + "-osm", + "Emit source map to the specified file", + WasmMergeOption, + Options::Arguments::One, + [&outputSourceMapFilename](Options* o, const std::string& argument) { + outputSourceMapFilename = argument; + }) + .add("--output-source-map-url", + "-osu", + "Emit specified string as source map URL", + WasmMergeOption, + Options::Arguments::One, + [&outputSourceMapUrl](Options* o, const std::string& argument) { + outputSourceMapUrl = argument; + }) .add("--rename-export-conflicts", "-rec", "Rename exports to avoid conflicts (rather than error)", @@ -529,6 +567,9 @@ Note that filenames and modules names are interleaved (which is hopefully less c for (Index i = 0; i < inputFiles.size(); i++) { auto inputFile = inputFiles[i]; auto inputFileName = inputFileNames[i]; + auto iter = inputSourceMapFilenames.find(i); + auto inputSourceMapFilename = + (iter == inputSourceMapFilenames.end()) ? "" : iter->second; if (options.debug) { std::cerr << "reading input '" << inputFile << "' as '" << inputFileName @@ -550,7 +591,7 @@ Note that filenames and modules names are interleaved (which is hopefully less c ModuleReader reader; try { - reader.read(inputFile, *currModule); + reader.read(inputFile, *currModule, inputSourceMapFilename); } catch (ParseException& p) { p.dump(std::cerr); Fatal() << "error in parsing wasm input: " << inputFile; @@ -606,6 +647,10 @@ Note that filenames and modules names are interleaved (which is hopefully less c ModuleWriter writer; writer.setBinary(emitBinary); writer.setDebugInfo(debugInfo); + if (outputSourceMapFilename.size()) { + writer.setSourceMapFilename(outputSourceMapFilename); + writer.setSourceMapUrl(outputSourceMapUrl); + } writer.write(merged, options.extra["output"]); } } diff --git a/test/lit/help/wasm-merge.test b/test/lit/help/wasm-merge.test index 50719f9882d..acd8d24509b 100644 --- a/test/lit/help/wasm-merge.test +++ b/test/lit/help/wasm-merge.test @@ -14,6 +14,11 @@ ;; CHECK-NEXT: ;; CHECK-NEXT: Note that filenames and modules names are interleaved (which is hopefully less ;; CHECK-NEXT: confusing). +;; CHECK-NEXT: +;; CHECK-NEXT: Input source maps can be specified by adding an --ism option right after the +;; CHECK-NEXT: module name: +;; CHECK-NEXT: +;; CHECK-NEXT: wasm-merge foo.wasm foo --ism foo.wasm.map ... ;; CHECK-NEXT: ================================================================================ ;; CHECK-NEXT: ;; CHECK-NEXT: @@ -22,6 +27,13 @@ ;; CHECK-NEXT: ;; CHECK-NEXT: --output,-o Output file (stdout if not specified) ;; CHECK-NEXT: +;; CHECK-NEXT: --input-source-map,-ism Consume source maps from the specified +;; CHECK-NEXT: file +;; CHECK-NEXT: +;; CHECK-NEXT: --output-source-map,-osm Emit source map to the specified file +;; CHECK-NEXT: +;; CHECK-NEXT: --output-source-map-url,-osu Emit specified string as source map URL +;; CHECK-NEXT: ;; CHECK-NEXT: --rename-export-conflicts,-rec Rename exports to avoid conflicts (rather ;; CHECK-NEXT: than error) ;; CHECK-NEXT: From 5a3caa454236f17f03cd07c69517fed738e6a4de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Vouillon?= Date: Wed, 28 Feb 2024 09:35:05 -0500 Subject: [PATCH 3/7] wasm-merge: copy sourcemap information --- src/ir/module-utils.cpp | 44 +++++++++++++++++++++++++++++++++++----- src/ir/module-utils.h | 15 +++++++++++--- src/tools/wasm-merge.cpp | 21 +++++++++++++++++-- 3 files changed, 70 insertions(+), 10 deletions(-) diff --git a/src/ir/module-utils.cpp b/src/ir/module-utils.cpp index b61fddd8b82..e3b5cff671e 100644 --- a/src/ir/module-utils.cpp +++ b/src/ir/module-utils.cpp @@ -15,6 +15,7 @@ */ #include "module-utils.h" +#include "ir/debug.h" #include "ir/intrinsics.h" #include "ir/manipulation.h" #include "ir/properties.h" @@ -23,17 +24,46 @@ namespace wasm::ModuleUtils { +static void updateLocationSet(std::set& locations, + std::vector* indexMap) { + std::set updatedLocations; + + for (auto iter : locations) { + iter.fileIndex = (*indexMap)[iter.fileIndex]; + updatedLocations.insert(iter); + } + locations.clear(); + std::swap(locations, updatedLocations); +} + // Copies a function into a module. If newName is provided it is used as the -// name of the function (otherwise the original name is copied). -Function* copyFunction(Function* func, Module& out, Name newName) { +// name of the function (otherwise the original name is copied). If indexMap is +// specified, it is used to rename source map filename indices when copying the +// function from one module to another one. +Function* copyFunction(Function* func, + Module& out, + Name newName, + std::vector* indexMap) { auto ret = std::make_unique(); ret->name = newName.is() ? newName : func->name; ret->type = func->type; ret->vars = func->vars; ret->localNames = func->localNames; ret->localIndices = func->localIndices; - ret->debugLocations = func->debugLocations; ret->body = ExpressionManipulator::copy(func->body, out); + if (ret->body) { + debug::copyDebugInfo(func->body, ret->body, func, ret.get()); + } + ret->prologLocation = func->prologLocation; + ret->epilogLocation = func->epilogLocation; + // Update file indices if needed + if (indexMap) { + for (auto& iter : ret->debugLocations) { + iter.second.fileIndex = (*indexMap)[iter.second.fileIndex]; + } + updateLocationSet(ret->prologLocation, indexMap); + updateLocationSet(ret->epilogLocation, indexMap); + } ret->module = func->module; ret->base = func->base; ret->noFullInline = func->noFullInline; @@ -135,9 +165,13 @@ DataSegment* copyDataSegment(const DataSegment* segment, Module& out) { // Copies named toplevel module items (things of kind ModuleItemKind). See // copyModule() for something that also copies exports, the start function, etc. -void copyModuleItems(const Module& in, Module& out) { +// The indexMap is used to update source map information when copying functions +// from one module to another. +void copyModuleItems(const Module& in, + Module& out, + std::vector* indexMap) { for (auto& curr : in.functions) { - copyFunction(curr.get(), out); + copyFunction(curr.get(), out, Name(), indexMap); } for (auto& curr : in.globals) { copyGlobal(curr.get(), out); diff --git a/src/ir/module-utils.h b/src/ir/module-utils.h index d33a47673ea..b4bf35d484b 100644 --- a/src/ir/module-utils.h +++ b/src/ir/module-utils.h @@ -24,8 +24,13 @@ namespace wasm::ModuleUtils { // Copies a function into a module. If newName is provided it is used as the -// name of the function (otherwise the original name is copied). -Function* copyFunction(Function* func, Module& out, Name newName = Name()); +// name of the function (otherwise the original name is copied). If indexMap is +// specified, it is used to rename source map filename indices when copying the +// function from one module to another one. +Function* copyFunction(Function* func, + Module& out, + Name newName = Name(), + std::vector* indexMap = 0); Global* copyGlobal(Global* global, Module& out); @@ -41,7 +46,11 @@ DataSegment* copyDataSegment(const DataSegment* segment, Module& out); // Copies named toplevel module items (things of kind ModuleItemKind). See // copyModule() for something that also copies exports, the start function, etc. -void copyModuleItems(const Module& in, Module& out); +// The indexMap is used to update source map information when copying functions +// from one module to another. +void copyModuleItems(const Module& in, + Module& out, + std::vector* indexMap = 0); void copyModule(const Module& in, Module& out); diff --git a/src/tools/wasm-merge.cpp b/src/tools/wasm-merge.cpp index ea7584f0aa8..931ed8638b5 100644 --- a/src/tools/wasm-merge.cpp +++ b/src/tools/wasm-merge.cpp @@ -324,9 +324,26 @@ void renameInputItems(Module& input) { } void copyModuleContents(Module& input, Name inputName) { - // First, copy the regular module items (functions, globals) etc. which we + // Source map filename index mapping + std::vector indexMap; + std::unordered_map debugInfoFileIndices; + for (Index i = 0; i < merged.debugInfoFileNames.size(); i++) { + debugInfoFileIndices[merged.debugInfoFileNames[i]] = i; + } + for (Index i = 0; i < input.debugInfoFileNames.size(); i++) { + std::string file = input.debugInfoFileNames[i]; + auto iter = debugInfoFileIndices.find(file); + if (iter == debugInfoFileIndices.end()) { + Index index = merged.debugInfoFileNames.size(); + merged.debugInfoFileNames.push_back(file); + debugInfoFileIndices[file] = index; + } + indexMap.push_back(debugInfoFileIndices[file]); + } + + // Copy the regular module items (functions, globals) etc. which we // have proper names for, and can just copy. - ModuleUtils::copyModuleItems(input, merged); + ModuleUtils::copyModuleItems(input, merged, &indexMap); // We must handle exports in a special way, as we need to note their origin // module as we copy them in (also, they are not importable or exportable, so From 824eda0b6d6a7e049547b8d989baf8309ca3988b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Vouillon?= Date: Wed, 28 Feb 2024 14:27:24 -0500 Subject: [PATCH 4/7] Add wasm-merge test --- test/lit/merge/sourcemap.wat | 33 +++++++++++++++++++++++++++++ test/lit/merge/sourcemap.wat.second | 8 +++++++ 2 files changed, 41 insertions(+) create mode 100644 test/lit/merge/sourcemap.wat create mode 100644 test/lit/merge/sourcemap.wat.second diff --git a/test/lit/merge/sourcemap.wat b/test/lit/merge/sourcemap.wat new file mode 100644 index 00000000000..f4c8d82ae68 --- /dev/null +++ b/test/lit/merge/sourcemap.wat @@ -0,0 +1,33 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. + +;; RUN: wasm-merge %s first %s.second second -S -o - | filecheck %s + +;; Test that sourcemap information is preserved + +(module + ;;@ a:1:1 + (func (export "f") + ;;@ a:2:1 + (nop) + ;;@ a:3:1 + ) +) +;; CHECK: (type $0 (func)) + +;; CHECK: (export "f" (func $0)) + +;; CHECK: (export "g" (func $0_1)) + +;; CHECK: ;;@ a:1:1 +;; CHECK-NEXT: (func $0 +;; CHECK-NEXT: ;;@ a:2:1 +;; CHECK-NEXT: (nop) +;; CHECK-NEXT: ;;@ a:3:1 +;; CHECK-NEXT: ) + +;; CHECK: ;;@ b:1:2 +;; CHECK-NEXT: (func $0_1 +;; CHECK-NEXT: ;;@ b:2:2 +;; CHECK-NEXT: (nop) +;; CHECK-NEXT: ;;@ b:3:2 +;; CHECK-NEXT: ) diff --git a/test/lit/merge/sourcemap.wat.second b/test/lit/merge/sourcemap.wat.second new file mode 100644 index 00000000000..0ea7c75fa03 --- /dev/null +++ b/test/lit/merge/sourcemap.wat.second @@ -0,0 +1,8 @@ +(module + ;;@ b:1:2 + (func (export "g") + ;;@ b:2:2 + (nop) + ;;@ b:3:2 + ) +) From ad6acace2a60d89c4bef698bce049d75e7314bce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Vouillon?= Date: Tue, 5 Mar 2024 04:32:15 +0100 Subject: [PATCH 5/7] Updates after review --- src/ir/module-utils.cpp | 46 +++++++++++++++--------- src/ir/module-utils.h | 21 +++++------ src/tools/wasm-merge.cpp | 21 ++--------- src/tools/wasm-metadce.cpp | 2 +- test/lit/merge/sourcemap.wat | 56 ++++++++++++++++++++--------- test/lit/metadce/sourcemap.wat | 26 ++++++++++++++ test/lit/metadce/sourcemap.wat.json | 13 +++++++ 7 files changed, 121 insertions(+), 64 deletions(-) create mode 100644 test/lit/metadce/sourcemap.wat create mode 100644 test/lit/metadce/sourcemap.wat.json diff --git a/src/ir/module-utils.cpp b/src/ir/module-utils.cpp index e3b5cff671e..549ed65502e 100644 --- a/src/ir/module-utils.cpp +++ b/src/ir/module-utils.cpp @@ -24,12 +24,14 @@ namespace wasm::ModuleUtils { +// Update the file name indices when moving a set of debug locations from one +// module to another. static void updateLocationSet(std::set& locations, - std::vector* indexMap) { + std::vector& fileIndexMap) { std::set updatedLocations; for (auto iter : locations) { - iter.fileIndex = (*indexMap)[iter.fileIndex]; + iter.fileIndex = fileIndexMap[iter.fileIndex]; updatedLocations.insert(iter); } locations.clear(); @@ -37,13 +39,13 @@ static void updateLocationSet(std::set& locations, } // Copies a function into a module. If newName is provided it is used as the -// name of the function (otherwise the original name is copied). If indexMap is -// specified, it is used to rename source map filename indices when copying the -// function from one module to another one. +// name of the function (otherwise the original name is copied). If fileIndexMap +// is specified, it is used to rename source map filename indices when copying +// the function from one module to another one. Function* copyFunction(Function* func, Module& out, Name newName, - std::vector* indexMap) { + std::optional> fileIndexMap) { auto ret = std::make_unique(); ret->name = newName.is() ? newName : func->name; ret->type = func->type; @@ -57,12 +59,12 @@ Function* copyFunction(Function* func, ret->prologLocation = func->prologLocation; ret->epilogLocation = func->epilogLocation; // Update file indices if needed - if (indexMap) { + if (fileIndexMap) { for (auto& iter : ret->debugLocations) { - iter.second.fileIndex = (*indexMap)[iter.second.fileIndex]; + iter.second.fileIndex = (*fileIndexMap)[iter.second.fileIndex]; } - updateLocationSet(ret->prologLocation, indexMap); - updateLocationSet(ret->epilogLocation, indexMap); + updateLocationSet(ret->prologLocation, *fileIndexMap); + updateLocationSet(ret->epilogLocation, *fileIndexMap); } ret->module = func->module; ret->base = func->base; @@ -165,13 +167,25 @@ DataSegment* copyDataSegment(const DataSegment* segment, Module& out) { // Copies named toplevel module items (things of kind ModuleItemKind). See // copyModule() for something that also copies exports, the start function, etc. -// The indexMap is used to update source map information when copying functions -// from one module to another. -void copyModuleItems(const Module& in, - Module& out, - std::vector* indexMap) { +void copyModuleItems(const Module& in, Module& out) { + std::vector fileIndexMap; + std::unordered_map debugInfoFileIndices; + for (Index i = 0; i < out.debugInfoFileNames.size(); i++) { + debugInfoFileIndices[out.debugInfoFileNames[i]] = i; + } + for (Index i = 0; i < in.debugInfoFileNames.size(); i++) { + std::string file = in.debugInfoFileNames[i]; + auto iter = debugInfoFileIndices.find(file); + if (iter == debugInfoFileIndices.end()) { + Index index = out.debugInfoFileNames.size(); + out.debugInfoFileNames.push_back(file); + debugInfoFileIndices[file] = index; + } + fileIndexMap.push_back(debugInfoFileIndices[file]); + } + for (auto& curr : in.functions) { - copyFunction(curr.get(), out, Name(), indexMap); + copyFunction(curr.get(), out, Name(), fileIndexMap); } for (auto& curr : in.globals) { copyGlobal(curr.get(), out); diff --git a/src/ir/module-utils.h b/src/ir/module-utils.h index b4bf35d484b..eabcd036c2b 100644 --- a/src/ir/module-utils.h +++ b/src/ir/module-utils.h @@ -24,13 +24,14 @@ namespace wasm::ModuleUtils { // Copies a function into a module. If newName is provided it is used as the -// name of the function (otherwise the original name is copied). If indexMap is -// specified, it is used to rename source map filename indices when copying the -// function from one module to another one. -Function* copyFunction(Function* func, - Module& out, - Name newName = Name(), - std::vector* indexMap = 0); +// name of the function (otherwise the original name is copied). If fileIndexMap +// is specified, it is used to rename source map filename indices when copying +// the function from one module to another one. +Function* +copyFunction(Function* func, + Module& out, + Name newName = Name(), + std::optional> fileIndexMap = std::nullopt); Global* copyGlobal(Global* global, Module& out); @@ -46,11 +47,7 @@ DataSegment* copyDataSegment(const DataSegment* segment, Module& out); // Copies named toplevel module items (things of kind ModuleItemKind). See // copyModule() for something that also copies exports, the start function, etc. -// The indexMap is used to update source map information when copying functions -// from one module to another. -void copyModuleItems(const Module& in, - Module& out, - std::vector* indexMap = 0); +void copyModuleItems(const Module& in, Module& out); void copyModule(const Module& in, Module& out); diff --git a/src/tools/wasm-merge.cpp b/src/tools/wasm-merge.cpp index 931ed8638b5..ea7584f0aa8 100644 --- a/src/tools/wasm-merge.cpp +++ b/src/tools/wasm-merge.cpp @@ -324,26 +324,9 @@ void renameInputItems(Module& input) { } void copyModuleContents(Module& input, Name inputName) { - // Source map filename index mapping - std::vector indexMap; - std::unordered_map debugInfoFileIndices; - for (Index i = 0; i < merged.debugInfoFileNames.size(); i++) { - debugInfoFileIndices[merged.debugInfoFileNames[i]] = i; - } - for (Index i = 0; i < input.debugInfoFileNames.size(); i++) { - std::string file = input.debugInfoFileNames[i]; - auto iter = debugInfoFileIndices.find(file); - if (iter == debugInfoFileIndices.end()) { - Index index = merged.debugInfoFileNames.size(); - merged.debugInfoFileNames.push_back(file); - debugInfoFileIndices[file] = index; - } - indexMap.push_back(debugInfoFileIndices[file]); - } - - // Copy the regular module items (functions, globals) etc. which we + // First, copy the regular module items (functions, globals) etc. which we // have proper names for, and can just copy. - ModuleUtils::copyModuleItems(input, merged, &indexMap); + ModuleUtils::copyModuleItems(input, merged); // We must handle exports in a special way, as we need to note their origin // module as we copy them in (also, they are not importable or exportable, so diff --git a/src/tools/wasm-metadce.cpp b/src/tools/wasm-metadce.cpp index 5acd9832c8c..cd8c8546a45 100644 --- a/src/tools/wasm-metadce.cpp +++ b/src/tools/wasm-metadce.cpp @@ -497,7 +497,7 @@ int main(int argc, const char* argv[]) { ModuleReader reader; reader.setDWARF(debugInfo); try { - reader.read(options.extra["infile"], wasm, input); + reader.read(options.extra["infile"], wasm, inputSourceMapFilename); } catch (ParseException& p) { p.dump(std::cerr); Fatal() << "error in parsing wasm input"; diff --git a/test/lit/merge/sourcemap.wat b/test/lit/merge/sourcemap.wat index f4c8d82ae68..7457d4307c4 100644 --- a/test/lit/merge/sourcemap.wat +++ b/test/lit/merge/sourcemap.wat @@ -1,6 +1,10 @@ ;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. -;; RUN: wasm-merge %s first %s.second second -S -o - | filecheck %s +;; RUN: wasm-merge %s first %s.second second -S -o - | filecheck %s --check-prefix=CHECK-TEXT +;; RUN: wasm-as %s -o %t.wasm --source-map %t.map +;; RUN: wasm-as %s.second -o %t.second.wasm --source-map %t.second.map +;; RUN: wasm-merge %t.wasm first --input-source-map %t.map %t.second.wasm second --input-source-map %t.second.map -o %t.merged.wasm --output-source-map %t.merged.map +;; RUN: wasm-dis %t.merged.wasm --source-map %t.merged.map -o - | filecheck %s --check-prefix=CHECK-BIN ;; Test that sourcemap information is preserved @@ -12,22 +16,42 @@ ;;@ a:3:1 ) ) -;; CHECK: (type $0 (func)) +;; CHECK-TEXT: (type $0 (func)) -;; CHECK: (export "f" (func $0)) +;; CHECK-TEXT: (export "f" (func $0)) -;; CHECK: (export "g" (func $0_1)) +;; CHECK-TEXT: (export "g" (func $0_1)) -;; CHECK: ;;@ a:1:1 -;; CHECK-NEXT: (func $0 -;; CHECK-NEXT: ;;@ a:2:1 -;; CHECK-NEXT: (nop) -;; CHECK-NEXT: ;;@ a:3:1 -;; CHECK-NEXT: ) +;; CHECK-TEXT: ;;@ a:1:1 +;; CHECK-TEXT-NEXT: (func $0 +;; CHECK-TEXT-NEXT: ;;@ a:2:1 +;; CHECK-TEXT-NEXT: (nop) +;; CHECK-TEXT-NEXT: ;;@ a:3:1 +;; CHECK-TEXT-NEXT: ) -;; CHECK: ;;@ b:1:2 -;; CHECK-NEXT: (func $0_1 -;; CHECK-NEXT: ;;@ b:2:2 -;; CHECK-NEXT: (nop) -;; CHECK-NEXT: ;;@ b:3:2 -;; CHECK-NEXT: ) +;; CHECK-TEXT: ;;@ b:1:2 +;; CHECK-TEXT-NEXT: (func $0_1 +;; CHECK-TEXT-NEXT: ;;@ b:2:2 +;; CHECK-TEXT-NEXT: (nop) +;; CHECK-TEXT-NEXT: ;;@ b:3:2 +;; CHECK-TEXT-NEXT: ) + +;; CHECK-BIN: (type $0 (func)) + +;; CHECK-BIN: (export "f" (func $0)) + +;; CHECK-BIN: (export "g" (func $1)) + +;; CHECK-BIN: ;;@ a:1:1 +;; CHECK-BIN-NEXT: (func $0 +;; CHECK-BIN-NEXT: ;;@ a:2:1 +;; CHECK-BIN-NEXT: (nop) +;; CHECK-BIN-NEXT: ;;@ a:3:1 +;; CHECK-BIN-NEXT: ) + +;; CHECK-BIN: ;;@ b:1:2 +;; CHECK-BIN-NEXT: (func $1 +;; CHECK-BIN-NEXT: ;;@ b:2:2 +;; CHECK-BIN-NEXT: (nop) +;; CHECK-BIN-NEXT: ;;@ b:3:2 +;; CHECK-BIN-NEXT: ) diff --git a/test/lit/metadce/sourcemap.wat b/test/lit/metadce/sourcemap.wat new file mode 100644 index 00000000000..bf695fd2139 --- /dev/null +++ b/test/lit/metadce/sourcemap.wat @@ -0,0 +1,26 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited. +;; RUN: wasm-metadce %s --graph-file %s.json -S -o - | filecheck %s +;; RUN: wasm-as %s -o %t.wasm --source-map %t.map +;; RUN: wasm-metadce %t.wasm --input-source-map %t.map --graph-file %s.json -o %t.out.wasm --output-source-map %t.out.map +;; RUN: wasm-dis %t.out.wasm --source-map %t.out.map -o - | filecheck %s + +;; Test that sourcemap information is preserved + +(module + ;;@ a:1:1 + (func (export "f") + ;;@ a:2:1 + (nop) + ;;@ a:3:1 + ) +) +;; CHECK: (type $0 (func)) + +;; CHECK: (export "f" (func $0)) + +;; CHECK: ;;@ a:1:1 +;; CHECK-NEXT: (func $0 +;; CHECK-NEXT: ;;@ a:2:1 +;; CHECK-NEXT: (nop) +;; CHECK-NEXT: ;;@ a:3:1 +;; CHECK-NEXT: ) diff --git a/test/lit/metadce/sourcemap.wat.json b/test/lit/metadce/sourcemap.wat.json new file mode 100644 index 00000000000..4b2028140e7 --- /dev/null +++ b/test/lit/metadce/sourcemap.wat.json @@ -0,0 +1,13 @@ +[ + { + "name": "root", + "reaches": [ + "f" + ], + "root": true + }, + { + "name": "f", + "export": "f" + } +] From 8ddd780b9a00fb1b44ba0ff235b421b35826f61e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Vouillon?= Date: Wed, 6 Mar 2024 04:00:25 +0100 Subject: [PATCH 6/7] Optimize the case when there is no debug information --- src/ir/debug.h | 4 ++++ src/ir/module-utils.cpp | 38 +++++++++++++++++++++----------------- src/passes/Inlining.cpp | 4 +--- 3 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/ir/debug.h b/src/ir/debug.h index 6dfd379d6c6..04838137e63 100644 --- a/src/ir/debug.h +++ b/src/ir/debug.h @@ -27,6 +27,10 @@ inline void copyDebugInfo(Expression* origin, Expression* copy, Function* originFunc, Function* copyFunc) { + if (originFunc->debugLocations.empty()) { + return; // No debug info to copy + } + struct Lister : public PostWalker> { std::vector list; void visitExpression(Expression* curr) { list.push_back(curr); } diff --git a/src/ir/module-utils.cpp b/src/ir/module-utils.cpp index 549ed65502e..852078930e9 100644 --- a/src/ir/module-utils.cpp +++ b/src/ir/module-utils.cpp @@ -53,9 +53,7 @@ Function* copyFunction(Function* func, ret->localNames = func->localNames; ret->localIndices = func->localIndices; ret->body = ExpressionManipulator::copy(func->body, out); - if (ret->body) { - debug::copyDebugInfo(func->body, ret->body, func, ret.get()); - } + debug::copyDebugInfo(func->body, ret->body, func, ret.get()); ret->prologLocation = func->prologLocation; ret->epilogLocation = func->epilogLocation; // Update file indices if needed @@ -168,20 +166,26 @@ DataSegment* copyDataSegment(const DataSegment* segment, Module& out) { // Copies named toplevel module items (things of kind ModuleItemKind). See // copyModule() for something that also copies exports, the start function, etc. void copyModuleItems(const Module& in, Module& out) { - std::vector fileIndexMap; - std::unordered_map debugInfoFileIndices; - for (Index i = 0; i < out.debugInfoFileNames.size(); i++) { - debugInfoFileIndices[out.debugInfoFileNames[i]] = i; - } - for (Index i = 0; i < in.debugInfoFileNames.size(); i++) { - std::string file = in.debugInfoFileNames[i]; - auto iter = debugInfoFileIndices.find(file); - if (iter == debugInfoFileIndices.end()) { - Index index = out.debugInfoFileNames.size(); - out.debugInfoFileNames.push_back(file); - debugInfoFileIndices[file] = index; - } - fileIndexMap.push_back(debugInfoFileIndices[file]); + // If the source module has some debug information, we first compute how + // to map file name indices from this modules to file name indices in + // the target module. + std::optional> fileIndexMap; + if (!in.debugInfoFileNames.empty()) { + std::unordered_map debugInfoFileIndices; + for (Index i = 0; i < out.debugInfoFileNames.size(); i++) { + debugInfoFileIndices[out.debugInfoFileNames[i]] = i; + } + fileIndexMap.emplace(); + for (Index i = 0; i < in.debugInfoFileNames.size(); i++) { + std::string file = in.debugInfoFileNames[i]; + auto iter = debugInfoFileIndices.find(file); + if (iter == debugInfoFileIndices.end()) { + Index index = out.debugInfoFileNames.size(); + out.debugInfoFileNames.push_back(file); + debugInfoFileIndices[file] = index; + } + fileIndexMap->push_back(debugInfoFileIndices[file]); + } } for (auto& curr : in.functions) { diff --git a/src/passes/Inlining.cpp b/src/passes/Inlining.cpp index 3ae63face6c..51dddaaa9bf 100644 --- a/src/passes/Inlining.cpp +++ b/src/passes/Inlining.cpp @@ -456,9 +456,7 @@ static Expression* doInlining(Module* module, } // Generate and update the inlined contents auto* contents = ExpressionManipulator::copy(from->body, *module); - if (!from->debugLocations.empty()) { - debug::copyDebugInfo(from->body, contents, from, into); - } + debug::copyDebugInfo(from->body, contents, from, into); updater.walk(contents); block->list.push_back(contents); block->type = retType; From b37f66c90773682cb535e9367895f85502123d82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Vouillon?= Date: Wed, 6 Mar 2024 16:40:43 +0100 Subject: [PATCH 7/7] Typo --- src/tools/wasm-merge.cpp | 4 ++-- test/lit/help/wasm-merge.test | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tools/wasm-merge.cpp b/src/tools/wasm-merge.cpp index ea7584f0aa8..ec67d56fc5a 100644 --- a/src/tools/wasm-merge.cpp +++ b/src/tools/wasm-merge.cpp @@ -469,9 +469,9 @@ will read foo.wasm and bar.wasm, with names 'foo' and 'bar' respectively, so if Note that filenames and modules names are interleaved (which is hopefully less confusing). -Input source maps can be specified by adding an --ism option right after the module name: +Input source maps can be specified by adding an -ism option right after the module name: - wasm-merge foo.wasm foo --ism foo.wasm.map ...)"); + wasm-merge foo.wasm foo -ism foo.wasm.map ...)"); options .add("--output", diff --git a/test/lit/help/wasm-merge.test b/test/lit/help/wasm-merge.test index acd8d24509b..7d55074c38d 100644 --- a/test/lit/help/wasm-merge.test +++ b/test/lit/help/wasm-merge.test @@ -15,10 +15,10 @@ ;; CHECK-NEXT: Note that filenames and modules names are interleaved (which is hopefully less ;; CHECK-NEXT: confusing). ;; CHECK-NEXT: -;; CHECK-NEXT: Input source maps can be specified by adding an --ism option right after the +;; CHECK-NEXT: Input source maps can be specified by adding an -ism option right after the ;; CHECK-NEXT: module name: ;; CHECK-NEXT: -;; CHECK-NEXT: wasm-merge foo.wasm foo --ism foo.wasm.map ... +;; CHECK-NEXT: wasm-merge foo.wasm foo -ism foo.wasm.map ... ;; CHECK-NEXT: ================================================================================ ;; CHECK-NEXT: ;; CHECK-NEXT: