[EH] Change Walker::TaskFunc back to function pointer (#3899)

`Walker::TaskFunc` has changed from a function pointer to
`std::function` in #3494, mainly to make the EH support for `CFGWalker`
easier. We didn't notice much performance difference then, but it was
recently reported that it creased binaryen.js code size and performance.
This changes `Walker::TaskFunc` back to a function pointer and does a
little more work to manage catch index in `CFGWalker` side.

Hopefully fixes #3857.

Author: aheejin

src/cfg/cfg-traversal.h

@@ -101,6 +101,10 @@ struct CFGWalker : public ControlFlowWalker<SubType, VisitorType> {
   // but their usage does not overlap in time, and this is more efficient.
   std::vector<std::vector<BasicBlock*>> processCatchStack;
 
+  // Stack to store the catch indices within catch bodies. To be used in
+  // doStartCatch and doEndCatch.
+  std::vector<Index> catchIndexStack;
+
   BasicBlock* startBasicBlock() {
     currBasicBlock = ((SubType*)this)->makeBasicBlock();
     basicBlocks.push_back(std::unique_ptr<BasicBlock>(currBasicBlock));
@@ -304,16 +308,20 @@ struct CFGWalker : public ControlFlowWalker<SubType, VisitorType> {
     self->processCatchStack.push_back(self->unwindCatchStack.back());
     self->unwindCatchStack.pop_back();
     self->unwindExprStack.pop_back();
+    self->catchIndexStack.push_back(0);
   }
 
-  static void doStartCatch(SubType* self, Expression** currp, Index i) {
+  static void doStartCatch(SubType* self, Expression** currp) {
     // Get the block that starts this catch
-    self->currBasicBlock = self->processCatchStack.back()[i];
+    self->currBasicBlock =
+      self->processCatchStack.back()[self->catchIndexStack.back()];
   }
 
-  static void doEndCatch(SubType* self, Expression** currp, Index i) {
+  static void doEndCatch(SubType* self, Expression** currp) {
     // We are done with this catch; set the block that ends it
-    self->processCatchStack.back()[i] = self->currBasicBlock;
+    self->processCatchStack.back()[self->catchIndexStack.back()] =
+      self->currBasicBlock;
+    self->catchIndexStack.back()++;
   }
 
   static void doEndTry(SubType* self, Expression** currp) {
@@ -326,6 +334,7 @@ struct CFGWalker : public ControlFlowWalker<SubType, VisitorType> {
     self->link(self->tryStack.back(), self->currBasicBlock);
     self->tryStack.pop_back();
     self->processCatchStack.pop_back();
+    self->catchIndexStack.pop_back();
   }
 
   static void doEndThrow(SubType* self, Expression** currp) {
@@ -381,17 +390,10 @@ struct CFGWalker : public ControlFlowWalker<SubType, VisitorType> {
       case Expression::Id::TryId: {
         self->pushTask(SubType::doEndTry, currp);
         auto& catchBodies = curr->cast<Try>()->catchBodies;
-        using namespace std::placeholders;
         for (Index i = 0; i < catchBodies.size(); i++) {
-          auto doEndCatchI = [i](SubType* self, Expression** currp) {
-            doEndCatch(self, currp, i);
-          };
-          self->pushTask(doEndCatchI, currp);
+          self->pushTask(doEndCatch, currp);
           self->pushTask(SubType::scan, &catchBodies[i]);
-          auto doStartCatchI = [i](SubType* self, Expression** currp) {
-            doStartCatch(self, currp, i);
-          };
-          self->pushTask(doStartCatchI, currp);
+          self->pushTask(doStartCatch, currp);
         }
         self->pushTask(SubType::doStartCatches, currp);
         self->pushTask(SubType::scan, &curr->cast<Try>()->body);

src/wasm-traversal.h

@@ -282,7 +282,7 @@ struct Walker : public VisitorType {
   // nested.
 
   // Tasks receive the this pointer and a pointer to the pointer to operate on
-  using TaskFunc = std::function<void(SubType*, Expression**)>;
+  typedef void (*TaskFunc)(SubType*, Expression**);
 
   struct Task {
     TaskFunc func;

test/passes/rse_all-features.txt

@@ -4,6 +4,7 @@
  (type $i32_i32_=>_none (func (param i32 i32)))
  (type $i32_f64_=>_none (func (param i32 f64)))
  (event $e (attr 0) (param i32))
+ (event $e2 (attr 0) (param))
  (func $basic (param $x i32) (param $y f64)
   (local $a f32)
   (local $b i64)
@@ -645,4 +646,84 @@
    (i32.const 1)
   )
  )
+ (func $nested-catch1
+  (local $x i32)
+  (try $try
+   (do
+    (throw $e
+     (i32.const 0)
+    )
+   )
+   (catch $e
+    (drop
+     (pop i32)
+    )
+   )
+   (catch $e2
+    (try $try6
+     (do
+      (throw $e
+       (i32.const 0)
+      )
+     )
+     (catch $e
+      (drop
+       (pop i32)
+      )
+     )
+     (catch $e2
+      (local.set $x
+       (i32.const 1)
+      )
+     )
+    )
+   )
+  )
+  (local.set $x
+   (i32.const 1)
+  )
+ )
+ (func $nested-catch2
+  (local $x i32)
+  (try $try
+   (do
+    (throw $e
+     (i32.const 0)
+    )
+   )
+   (catch $e
+    (drop
+     (pop i32)
+    )
+    (local.set $x
+     (i32.const 1)
+    )
+   )
+   (catch_all
+    (try $try7
+     (do
+      (throw $e
+       (i32.const 0)
+      )
+     )
+     (catch $e
+      (drop
+       (pop i32)
+      )
+      (local.set $x
+       (i32.const 1)
+      )
+     )
+     (catch_all
+      (local.set $x
+       (i32.const 1)
+      )
+     )
+    )
+   )
+  )
+  (drop
+   (i32.const 1)
+  )
+ )
 )

test/passes/rse_all-features.wast

@@ -288,6 +288,7 @@
   )
 
   (event $e (attr 0) (param i32))
+  (event $e2 (attr 0))
   (func $try1
     (local $x i32)
     (try
@@ -416,4 +417,61 @@
     ;; so the local.set may not run. So this should NOT be dropped.
     (local.set $x (i32.const 1))
   )
+  (func $nested-catch1
+    (local $x i32)
+    (try
+      (do
+        (throw $e (i32.const 0))
+      )
+      (catch $e
+        (drop (pop i32))
+      )
+      (catch $e2
+        (try
+          (do
+            (throw $e (i32.const 0))
+          )
+          (catch $e
+            (drop (pop i32))
+          )
+          (catch $e2
+            (local.set $x (i32.const 1))
+          )
+        )
+      )
+    )
+    ;; This should NOT be dropped because the exception might not be caught by
+    ;; the inner catches, and the local.set above us may not have run, and
+    ;; other possible code paths do not even set the local.
+    (local.set $x (i32.const 1))
+  )
+  (func $nested-catch2
+    (local $x i32)
+    (try
+      (do
+        (throw $e (i32.const 0))
+      )
+      (catch $e
+        (drop (pop i32))
+        (local.set $x (i32.const 1))
+      )
+      (catch_all
+        (try
+          (do
+            (throw $e (i32.const 0))
+          )
+          (catch $e
+            (drop (pop i32))
+            (local.set $x (i32.const 1))
+          )
+          (catch_all
+            (local.set $x (i32.const 1))
+          )
+        )
+      )
+    )
+    ;; This should be dropped because the exception is guaranteed to be caught
+    ;; by one of the catches and it will set the local to 1.
+    (local.set $x (i32.const 1))
+  )
 )