[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.
author: Alon Zakai <azakai@google.com> 2024-06-12 11:05:28 -0700
committer: GitHub <noreply@github.com> 2024-06-12 11:05:28 -0700
commit: ac21c8cc9204e09fab070d2fd915e7ab583a5dac (patch)
tree: 190a13113804b3c6ea01dd71f44694e1b6111be0 /src
parent: 475841dd5f0c50d7072f6dfc26dffd66e02abc10 (diff)
download: binaryen-ac21c8cc9204e09fab070d2fd915e7ab583a5dac.tar.gz
binaryen-ac21c8cc9204e09fab070d2fd915e7ab583a5dac.tar.bz2
binaryen-ac21c8cc9204e09fab070d2fd915e7ab583a5dac.zip
7 files changed, 97 insertions, 52 deletions
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/debuginfo.cpp
index 04838137e..a5fe92d54 100644
--- a/src/ir/debug.h
+++ b/src/ir/debuginfo.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright 2019 WebAssembly Community Group participants
+ * 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.
@@ -14,16 +14,13 @@
  * limitations under the License.
  */
 
-#ifndef wasm_ir_debug_h
-#define wasm_ir_debug_h
+#include "ir/debuginfo.h"
+#include "wasm-traversal.h"
+#include "wasm.h"
 
-#include <wasm-traversal.h>
+namespace wasm::debuginfo {
 
-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,
+void copyBetweenFunctions(Expression* origin,
                           Expression* copy,
                           Function* originFunc,
                           Function* copyFunc) {
@@ -52,8 +49,6 @@ inline void copyDebugInfo(Expression* origin,
       copyDebug[copyList.list[i]] = location;
     }
   }
-};
-
-} // namespace wasm::debug
+}
 
-#endif // wasm_ir_debug_h
+} // 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 <atomic>
 
 #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 <variant>
 
+#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<Trap>(&info)) {
-      return builder.makeUnreachable();
+      ret = builder.makeUnreachable();
     } else {
-      return builder.makeCall(std::get<Known>(info).target,
-                              getOperands(),
-                              curr->type,
-                              curr->isReturn);
+      ret = builder.makeCall(std::get<Known>(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;
   }
author	Alon Zakai <azakai@google.com>	2024-06-12 11:05:28 -0700
committer	GitHub <noreply@github.com>	2024-06-12 11:05:28 -0700
commit	ac21c8cc9204e09fab070d2fd915e7ab583a5dac (patch)
tree	190a13113804b3c6ea01dd71f44694e1b6111be0 /src
parent	475841dd5f0c50d7072f6dfc26dffd66e02abc10 (diff)
download	binaryen-ac21c8cc9204e09fab070d2fd915e7ab583a5dac.tar.gz binaryen-ac21c8cc9204e09fab070d2fd915e7ab583a5dac.tar.bz2 binaryen-ac21c8cc9204e09fab070d2fd915e7ab583a5dac.zip