From ac21c8cc9204e09fab070d2fd915e7ab583a5dac Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 12 Jun 2024 11:05:28 -0700 Subject: [DebugInfo] Copy debug info in call-utils.h (#6652) We automatically copy debuginfo in replaceCurrent(), but there are a few places that do other operations than simple replacements. call-utils.h will turn a call_ref with a select target into two direct calls, and we were missing the logic to copy debuginfo from the call_ref to the calls. To make this work, refactor out the copying logic from wasm-traversal, into debuginfo.h, and use it in call-utils.h. debuginfo.h itself is renamed from debug.h (as now this needs to be included from wasm-traversal, which nearly everything does, and it turns out some files have internal stuff like a debug() helper that ends up conflicing with the old debug namespace). Also rename the old copyDebugInfo function to copyDebugInfoBetweenFunctions which is more explicit. That is also moved from the header to a cpp file because it depends on wasm-traversal (so we'd end up with recursive headers otherwise). That is fine, as that method is called after copying a function, which is not that frequent. The new copyDebugInfoToReplacement (which was refactored out of wasm-traversal) is in the header because it can be called very frequently (every single instruction we optimize) and we want it to get inlined. --- src/ir/CMakeLists.txt | 1 + src/ir/debug.h | 59 ---------------------------------------- src/ir/debuginfo.cpp | 54 +++++++++++++++++++++++++++++++++++++ src/ir/debuginfo.h | 72 +++++++++++++++++++++++++++++++++++++++++++++++++ src/ir/module-utils.cpp | 4 +-- src/passes/Inlining.cpp | 4 +-- src/passes/call-utils.h | 14 ++++++---- src/wasm-traversal.h | 33 +++-------------------- 8 files changed, 143 insertions(+), 98 deletions(-) delete mode 100644 src/ir/debug.h create mode 100644 src/ir/debuginfo.cpp create mode 100644 src/ir/debuginfo.h (limited to 'src') diff --git a/src/ir/CMakeLists.txt b/src/ir/CMakeLists.txt index 279a1468c..996daa564 100644 --- a/src/ir/CMakeLists.txt +++ b/src/ir/CMakeLists.txt @@ -2,6 +2,7 @@ FILE(GLOB ir_HEADERS *.h) set(ir_SOURCES ExpressionAnalyzer.cpp ExpressionManipulator.cpp + debuginfo.cpp drop.cpp effects.cpp eh-utils.cpp diff --git a/src/ir/debug.h b/src/ir/debug.h deleted file mode 100644 index 04838137e..000000000 --- a/src/ir/debug.h +++ /dev/null @@ -1,59 +0,0 @@ -/* - * Copyright 2019 WebAssembly Community Group participants - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#ifndef wasm_ir_debug_h -#define wasm_ir_debug_h - -#include - -namespace wasm::debug { - -// Given an expression and a copy of it in another function, copy the debug -// info into the second function. -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); } - }; - - Lister originList; - originList.walk(origin); - Lister copyList; - copyList.walk(copy); - - auto& originDebug = originFunc->debugLocations; - auto& copyDebug = copyFunc->debugLocations; - - assert(originList.list.size() == copyList.list.size()); - for (Index i = 0; i < originList.list.size(); i++) { - auto iter = originDebug.find(originList.list[i]); - if (iter != originDebug.end()) { - auto location = iter->second; - copyDebug[copyList.list[i]] = location; - } - } -}; - -} // namespace wasm::debug - -#endif // wasm_ir_debug_h diff --git a/src/ir/debuginfo.cpp b/src/ir/debuginfo.cpp new file mode 100644 index 000000000..a5fe92d54 --- /dev/null +++ b/src/ir/debuginfo.cpp @@ -0,0 +1,54 @@ +/* + * Copyright 2024 WebAssembly Community Group participants + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "ir/debuginfo.h" +#include "wasm-traversal.h" +#include "wasm.h" + +namespace wasm::debuginfo { + +void copyBetweenFunctions(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); } + }; + + Lister originList; + originList.walk(origin); + Lister copyList; + copyList.walk(copy); + + auto& originDebug = originFunc->debugLocations; + auto& copyDebug = copyFunc->debugLocations; + + assert(originList.list.size() == copyList.list.size()); + for (Index i = 0; i < originList.list.size(); i++) { + auto iter = originDebug.find(originList.list[i]); + if (iter != originDebug.end()) { + auto location = iter->second; + copyDebug[copyList.list[i]] = location; + } + } +} + +} // namespace wasm::debuginfo diff --git a/src/ir/debuginfo.h b/src/ir/debuginfo.h new file mode 100644 index 000000000..96c4d8c2a --- /dev/null +++ b/src/ir/debuginfo.h @@ -0,0 +1,72 @@ +/* + * Copyright 2019 WebAssembly Community Group participants + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef wasm_ir_debuginfo_h +#define wasm_ir_debuginfo_h + +#include "wasm.h" + +namespace wasm::debuginfo { + +// Given an original expression and another that replaces it, copy the debuginfo +// from the former to the latter. Note the expression may not be an exclusive +// replacement of the other (the other may be replaced by several expressions, +// all of whom may end up with the same debug info). +inline void copyOriginalToReplacement(Expression* original, + Expression* replacement, + Function* func) { + auto& debugLocations = func->debugLocations; + // Early exit if there is no debug info at all. Also, leave if we already + // have debug info on the new replacement, which we don't want to trample: + // if there is no debug info we do want to copy, as a replacement operation + // suggests the new code plays the same role (it is an optimized version of + // the old), but if the code is already annotated, trust that. + if (debugLocations.empty() || debugLocations.count(replacement)) { + return; + } + + auto iter = debugLocations.find(original); + if (iter != debugLocations.end()) { + debugLocations[replacement] = iter->second; + // Note that we do *not* erase the debug info of the expression being + // replaced, because it may still exist: we might replace + // + // (call + // (block .. + // + // with + // + // (block + // (call .. + // + // We still want the call here to have its old debug info. + // + // (In most cases, of course, we do remove the replaced expression, + // which means we accumulate unused garbage in debugLocations, but + // that's not that bad; we use arena allocation for Expressions, after + // all.) + } +} + +// Given an expression and a copy of it in another function, copy the debug +// info into the second function. +void copyBetweenFunctions(Expression* origin, + Expression* copy, + Function* originFunc, + Function* copyFunc); +} // namespace wasm::debuginfo + +#endif // wasm_ir_debuginfo_h diff --git a/src/ir/module-utils.cpp b/src/ir/module-utils.cpp index 99652ceff..5791ed77c 100644 --- a/src/ir/module-utils.cpp +++ b/src/ir/module-utils.cpp @@ -15,7 +15,7 @@ */ #include "module-utils.h" -#include "ir/debug.h" +#include "ir/debuginfo.h" #include "ir/intrinsics.h" #include "ir/manipulation.h" #include "ir/properties.h" @@ -54,7 +54,7 @@ Function* copyFunction(Function* func, ret->localNames = func->localNames; ret->localIndices = func->localIndices; ret->body = ExpressionManipulator::copy(func->body, out); - debug::copyDebugInfo(func->body, ret->body, func, ret.get()); + debuginfo::copyBetweenFunctions(func->body, ret->body, func, ret.get()); ret->prologLocation = func->prologLocation; ret->epilogLocation = func->epilogLocation; // Update file indices if needed diff --git a/src/passes/Inlining.cpp b/src/passes/Inlining.cpp index 04bc6d78a..9e0d3eb67 100644 --- a/src/passes/Inlining.cpp +++ b/src/passes/Inlining.cpp @@ -31,7 +31,7 @@ #include #include "ir/branch-utils.h" -#include "ir/debug.h" +#include "ir/debuginfo.h" #include "ir/drop.h" #include "ir/eh-utils.h" #include "ir/element-utils.h" @@ -579,7 +579,7 @@ static Expression* doInlining(Module* module, // Generate and update the inlined contents auto* contents = ExpressionManipulator::copy(from->body, *module); - debug::copyDebugInfo(from->body, contents, from, into); + debuginfo::copyBetweenFunctions(from->body, contents, from, into); updater.walk(contents); block->list.push_back(contents); block->type = retType; diff --git a/src/passes/call-utils.h b/src/passes/call-utils.h index 7f2ebfda3..10106a756 100644 --- a/src/passes/call-utils.h +++ b/src/passes/call-utils.h @@ -19,6 +19,7 @@ #include +#include "ir/debuginfo.h" #include "ir/type-updating.h" #include "wasm.h" @@ -130,14 +131,17 @@ convertToDirectCalls(T* curr, }; auto makeCall = [&](IndirectCallInfo info) -> Expression* { + Expression* ret; if (std::get_if(&info)) { - return builder.makeUnreachable(); + ret = builder.makeUnreachable(); } else { - return builder.makeCall(std::get(info).target, - getOperands(), - curr->type, - curr->isReturn); + ret = builder.makeCall(std::get(info).target, + getOperands(), + curr->type, + curr->isReturn); } + debuginfo::copyOriginalToReplacement(curr, ret, &func); + return ret; }; auto* ifTrueCall = makeCall(ifTrueCallInfo); auto* ifFalseCall = makeCall(ifFalseCallInfo); diff --git a/src/wasm-traversal.h b/src/wasm-traversal.h index 8e99749b6..db9d1c1c4 100644 --- a/src/wasm-traversal.h +++ b/src/wasm-traversal.h @@ -27,6 +27,7 @@ #ifndef wasm_wasm_traversal_h #define wasm_wasm_traversal_h +#include "ir/debuginfo.h" #include "support/small_vector.h" #include "support/threads.h" #include "wasm.h" @@ -123,36 +124,8 @@ struct Walker : public VisitorType { Expression* replaceCurrent(Expression* expression) { // Copy debug info, if present. if (currFunction) { - auto& debugLocations = currFunction->debugLocations; - // Early exit if there is no debug info at all. Also, leave if we already - // have debug info on the new expression, which we don't want to trample: - // if there is no debug info we do want to copy, as a replacement - // operation suggests the new code plays the same role (it is an optimized - // version of the old), but if the code is already annotated, trust that. - if (!debugLocations.empty() && !debugLocations.count(expression)) { - auto* curr = getCurrent(); - auto iter = debugLocations.find(curr); - if (iter != debugLocations.end()) { - debugLocations[expression] = iter->second; - // Note that we do *not* erase the debug info of the expression being - // replaced, because it may still exist: we might replace - // - // (call - // (block .. - // - // with - // - // (block - // (call .. - // - // We still want the call here to have its old debug info. - // - // (In most cases, of course, we do remove the replaced expression, - // which means we accumulate unused garbage in debugLocations, but - // that's not that bad; we use arena allocation for Expressions, after - // all.) - } - } + debuginfo::copyOriginalToReplacement( + getCurrent(), expression, currFunction); } return *replacep = expression; } -- cgit v1.2.3