fix: source map generation and print with depth

WebAssembly · May 23, 2023 · 50e1f1b · 50e1f1b
1 parent 5beebc6
commit 50e1f1b
Show file tree

Hide file tree

Showing 6 changed files with 126 additions and 15 deletions.
diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp
@@ -18,6 +18,7 @@
 // Print out text in s-expression format
 //
 
+#include <cstdint>
 #include <ir/iteration.h>
 #include <ir/module-utils.h>
 #include <ir/table-utils.h>
@@ -2532,7 +2533,7 @@ struct PrintExpressionContents
 // internal contents and the nested children.
 struct PrintSExpression : public UnifiedExpressionVisitor<PrintSExpression> {
   std::ostream& o;
-  unsigned indent = 0;
+  uint32_t indent = 0U;
 
   bool minify;
   const char* maybeSpace;
@@ -2547,6 +2548,10 @@ struct PrintSExpression : public UnifiedExpressionVisitor<PrintSExpression> {
   Module* currModule = nullptr;
   Function* currFunction = nullptr;
   Function::DebugLocation lastPrintedLocation;
+  uint32_t lastPrintedIndent =
+    static_cast<uint32_t>(-1); ///< last print indent, child will not duplicate
+                               ///< the debug location, siblings will not miss
+                               ///< the debug location
   bool debugInfo;
 
   // Used to print delegate's depth argument when it throws to the caller
@@ -2560,10 +2565,17 @@ struct PrintSExpression : public UnifiedExpressionVisitor<PrintSExpression> {
   }
 
   void printDebugLocation(const Function::DebugLocation& location) {
-    if (lastPrintedLocation == location) {
+    // ;; debug location
+    // (parent
+    //  (child) ;; no debug location
+    // )
+    // ;; debug location
+    // (siblings)
+    if (lastPrintedLocation == location && indent > lastPrintedIndent) {
       return;
     }
     lastPrintedLocation = location;
+    lastPrintedIndent = indent;
     auto fileName = currModule->debugInfoFileNames[location.fileIndex];
     o << ";;@ " << fileName << ":" << location.lineNumber << ":"
       << location.columnNumber << '\n';

diff --git a/src/wasm-binary.h b/src/wasm-binary.h
@@ -1343,7 +1343,6 @@ class WasmBinaryWriter {
   void writeDylinkSection();
   void writeLegacyDylinkSection();
 
-  void initializeDebugInfo();
   void writeSourceMapProlog();
   void writeSourceMapEpilog();
   void writeDebugLocation(const Function::DebugLocation& loc);
@@ -1404,7 +1403,6 @@ class WasmBinaryWriter {
   std::vector<std::pair<size_t, const Function::DebugLocation*>>
     sourceMapLocations;
   size_t sourceMapLocationsSizeAtSectionStart;
-  Function::DebugLocation lastDebugLocation;
 
   std::unique_ptr<ImportInfo> importInfo;
 

diff --git a/src/wasm-stack.h b/src/wasm-stack.h
@@ -24,6 +24,7 @@
 #include "wasm-binary.h"
 #include "wasm-traversal.h"
 #include "wasm.h"
+#include <cstdint>
 
 namespace wasm {
 
@@ -154,7 +155,10 @@ class BinaryInstWriter : public OverriddenVisitor<BinaryInstWriter> {
 template<typename SubType>
 class BinaryenIRWriter : public Visitor<BinaryenIRWriter<SubType>> {
 public:
-  BinaryenIRWriter(Function* func) : func(func) {}
+  BinaryenIRWriter(Function* func)
+    : func(func), lastDebugLocation({static_cast<uint32_t>(-1),
+                                     static_cast<uint32_t>(-1),
+                                     static_cast<uint32_t>(-1)}) {}
 
   void write();
 
@@ -168,6 +172,10 @@ class BinaryenIRWriter : public Visitor<BinaryenIRWriter<SubType>> {
 
 protected:
   Function* func = nullptr;
+  Function::DebugLocation lastDebugLocation; ///< last debug location
+  uint32_t lastDebugLocationDepth =
+    static_cast<uint32_t>(-1); ///< last debug location depth
+  uint32_t depth = 0U;         ///< depth of current write cursor
 
 private:
   void emit(Expression* curr) { static_cast<SubType*>(this)->emit(curr); }
@@ -239,6 +247,7 @@ void BinaryenIRWriter<SubType>::visit(Expression* curr) {
   // unreachable block is a source of unreachability, which means we don't need
   // to emit an extra `unreachable` before the end of the block to prevent type
   // errors.
+  depth++;
   bool hasUnreachableChild = false;
   for (auto* child : ValueChildIterator(curr)) {
     visit(child);
@@ -251,7 +260,22 @@ void BinaryenIRWriter<SubType>::visit(Expression* curr) {
     // `curr` is not reachable, so don't emit it.
     return;
   }
-  emitDebugLocation(curr);
+
+  if (func != nullptr) {
+    const auto& debugLocationIterator = func->debugLocations.find(curr);
+    if (debugLocationIterator != func->debugLocations.cend()) {
+      if (lastDebugLocation != debugLocationIterator->second ||
+          depth < lastDebugLocationDepth) {
+        emitDebugLocation(curr);
+        lastDebugLocationDepth = depth;
+        lastDebugLocation = debugLocationIterator->second;
+      }
+    }
+  } else {
+    emitDebugLocation(
+      curr); // for global expression write, no func value, directly write it
+  }
+  depth--;
   // Control flow requires special handling, but most instructions can be
   // emitted directly after their children.
   if (Properties::isControlFlowStructure(curr)) {

diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
@@ -43,7 +43,6 @@ void WasmBinaryWriter::write() {
 
   writeDylinkSection();
 
-  initializeDebugInfo();
   if (sourceMap) {
     writeSourceMapProlog();
   }
@@ -1120,10 +1119,6 @@ void WasmBinaryWriter::writeSymbolMap() {
   file.close();
 }
 
-void WasmBinaryWriter::initializeDebugInfo() {
-  lastDebugLocation = {0, /* lineNumber = */ 1, 0};
-}
-
 void WasmBinaryWriter::writeSourceMapProlog() {
   *sourceMap << "{\"version\":3,\"sources\":[";
   for (size_t i = 0; i < wasm->debugInfoFileNames.size(); i++) {
@@ -1304,12 +1299,8 @@ void WasmBinaryWriter::writeDylinkSection() {
 }
 
 void WasmBinaryWriter::writeDebugLocation(const Function::DebugLocation& loc) {
-  if (loc == lastDebugLocation) {
-    return;
-  }
   auto offset = o.size();
   sourceMapLocations.emplace_back(offset, &loc);
-  lastDebugLocation = loc;
 }
 
 void WasmBinaryWriter::writeDebugLocation(Expression* curr, Function* func) {

diff --git a/test/fib-dbg.wasm.fromBinary b/test/fib-dbg.wasm.fromBinary
@@ -133,6 +133,7 @@
      (i32.const 0)
     )
    )
+   ;;@ fib.c:3:0
    (if
     (local.get $6)
     (block
@@ -156,6 +157,7 @@
      )
     )
    )
+   ;;@ fib.c:8:0
    (loop $label$4
     (block $label$5
      ;;@ fib.c:4:0
@@ -172,12 +174,14 @@
        (i32.const 1)
       )
      )
+     ;;@ fib.c:3:0
      (local.set $7
       (i32.eq
        (local.get $9)
        (local.get $0)
       )
      )
+     ;;@ fib.c:3:0
      (if
       (local.get $7)
       (block
@@ -201,6 +205,7 @@
        )
       )
      )
+     ;;@ fib.c:3:0
      (br $label$4)
     )
    )

diff --git a/test/lit/source-map-siblings-print.wast b/test/lit/source-map-siblings-print.wast
@@ -0,0 +1,81 @@
+;; RUN: wasm-opt %s -o %t.wasm -osm %t.map -g -q
+;; RUN: wasm-opt %t.wasm -ism %t.map -q -o - -S | filecheck %s
+
+(module
+  (func $foo (param $x i32) (param $y i32)
+    ;;@ src.cpp:1:1
+    (if
+      (i32.add
+        (local.get $x)
+        ;;@ src.cpp:2:1
+        (local.get $y)
+      )
+      ;;@ src.cpp:3:1
+      (return)
+    )
+    ;;@ src.cpp:1:1
+    (block
+      ;;@ src.cpp:4:1
+      (drop
+        (i32.add
+          (local.get $x)
+          ;;@ src.cpp:5:1
+          (local.get $y)
+        )
+      )
+      (drop
+        (i32.sub
+          (local.get $x)
+          (local.get $y)
+        )
+      )
+    )
+    (drop
+      (i32.mul
+        (local.get $x)
+        ;;@ src.cpp:6:1
+        (local.get $y)
+      )
+    )
+    (return)
+  )
+)
+
+;; CHECK:  (func $foo (param $x i32) (param $y i32)
+;; CHECK-NEXT:  ;;@ src.cpp:1:1
+;; CHECK-NEXT:  (if
+;; CHECK-NEXT:   (i32.add
+;; CHECK-NEXT:    (local.get $x)
+;; CHECK-NEXT:    ;;@ src.cpp:2:1
+;; CHECK-NEXT:    (local.get $y)
+;; CHECK-NEXT:   )
+;; CHECK-NEXT:   ;;@ src.cpp:3:1
+;; CHECK-NEXT:   (return)
+;; CHECK-NEXT:  )
+;; CHECK-NEXT:  ;;@ src.cpp:4:1
+;; CHECK-NEXT:  (drop
+;; CHECK-NEXT:   (i32.add
+;; CHECK-NEXT:    (local.get $x)
+;; CHECK-NEXT:    ;;@ src.cpp:5:1
+;; CHECK-NEXT:    (local.get $y)
+;; CHECK-NEXT:   )
+;; CHECK-NEXT:  )
+;; CHECK-NEXT:  ;;@ src.cpp:4:1
+;; CHECK-NEXT:  (drop
+;; CHECK-NEXT:   (i32.sub
+;; CHECK-NEXT:    (local.get $x)
+;; CHECK-NEXT:    (local.get $y)
+;; CHECK-NEXT:   )
+;; CHECK-NEXT:  )
+;; CHECK-NEXT:  ;;@ src.cpp:1:1
+;; CHECK-NEXT:  (drop
+;; CHECK-NEXT:   (i32.mul
+;; CHECK-NEXT:    (local.get $x)
+;; CHECK-NEXT:    ;;@ src.cpp:6:1
+;; CHECK-NEXT:    (local.get $y)
+;; CHECK-NEXT:   )
+;; CHECK-NEXT:  )
+;; CHECK-NEXT:  ;;@ src.cpp:1:1
+;; CHECK-NEXT:  (return)
+;; CHECK-NEXT: )
+;; CHECK-NEXT:)