More #1678 fixes (#1685)

While debugging to fix the waterfall regressions I noticed that wasm-reduce regressed. We need to be more careful with visitFunction which now may visit an imported function - I found a few not-well-tested passes that also regressed that way.

Author: kripken

src/passes/I64ToI32Lowering.cpp

@@ -175,6 +175,9 @@ struct I64ToI32Lowering : public WalkerPass<PostWalker<I64ToI32Lowering>> {
   }
 
   void visitFunction(Function* func) {
+    if (func->imported()) {
+      return;
+    }
     if (func->result == i64) {
       func->result = i32;
       // body may not have out param if it ends with control flow

src/passes/LogExecution.cpp

@@ -42,6 +42,9 @@ struct LogExecution : public WalkerPass<PostWalker<LogExecution>> {
   }
 
   void visitFunction(Function* curr) {
+    if (curr->imported()) {
+      return;
+    }
     curr->body = makeLogCall(curr->body);
   }
 

src/tools/wasm-reduce.cpp

@@ -524,9 +524,11 @@ struct Reducer : public WalkerPass<PostWalker<Reducer, UnifiedExpressionVisitor<
   }
 
   void visitFunction(Function* curr) {
-    // extra chance to work on the function toplevel element, as if it can
-    // be reduced it's great
-    visitExpression(curr->body);
+    if (!curr->imported()) {
+      // extra chance to work on the function toplevel element, as if it can
+      // be reduced it's great
+      visitExpression(curr->body);
+    }
     // finish function
     funcsSeen++;
     static int last = 0;
@@ -677,7 +679,7 @@ struct Reducer : public WalkerPass<PostWalker<Reducer, UnifiedExpressionVisitor<
     if (module->functions.size() == 1 && module->exports.empty() && module->table.segments.empty()) {
       auto* func = module->functions[0].get();
       // We can't remove something that might have breaks to it.
-      if (!Properties::isNamedControlFlow(func->body)) {
+      if (!func->imported() && !Properties::isNamedControlFlow(func->body)) {
         auto funcType = func->type;
         auto funcResult = func->result;
         auto* funcBody = func->body;

src/wasm/wasm-emscripten.cpp

@@ -489,6 +489,9 @@ struct EmJsWalker : public PostWalker<EmJsWalker> {
       segmentOffsets(getSegmentOffsets(wasm)) { }
 
   void visitFunction(Function* curr) {
+    if (curr->imported()) {
+      return;
+    }
     if (!curr->name.startsWith(EM_JS_PREFIX.str)) {
       return;
     }

test/passes/i64-to-i32-lowering.txt

@@ -0,0 +1,62 @@
+(module
+ (type $FUNCSIG$j (func (result i32)))
+ (import "env" "func" (func $import (result i32)))
+ (global $i64toi32_i32$HIGH_BITS (mut i32) (i32.const 0))
+ (func $defined (; 1 ;) (type $FUNCSIG$j) (result i32)
+  (local $i64toi32_i32$0 i32)
+  (local $i64toi32_i32$1 i32)
+  (local $i64toi32_i32$2 i32)
+  (local $i64toi32_i32$3 i32)
+  (local $i64toi32_i32$4 i32)
+  (local $i64toi32_i32$5 i32)
+  (set_local $i64toi32_i32$2
+   (block (result i32)
+    (set_local $i64toi32_i32$2
+     (block (result i32)
+      (set_local $i64toi32_i32$0
+       (i32.const 0)
+      )
+      (i32.const 1)
+     )
+    )
+    (set_local $i64toi32_i32$3
+     (block (result i32)
+      (set_local $i64toi32_i32$1
+       (i32.const 0)
+      )
+      (i32.const 2)
+     )
+    )
+    (set_local $i64toi32_i32$4
+     (i32.add
+      (get_local $i64toi32_i32$2)
+      (get_local $i64toi32_i32$3)
+     )
+    )
+    (set_local $i64toi32_i32$5
+     (i32.add
+      (get_local $i64toi32_i32$0)
+      (get_local $i64toi32_i32$1)
+     )
+    )
+    (if
+     (i32.lt_u
+      (get_local $i64toi32_i32$4)
+      (get_local $i64toi32_i32$3)
+     )
+     (set_local $i64toi32_i32$5
+      (i32.add
+       (get_local $i64toi32_i32$5)
+       (i32.const 1)
+      )
+     )
+    )
+    (get_local $i64toi32_i32$4)
+   )
+  )
+  (set_global $i64toi32_i32$HIGH_BITS
+   (get_local $i64toi32_i32$5)
+  )
+  (get_local $i64toi32_i32$2)
+ )
+)

test/passes/i64-to-i32-lowering.wast

@@ -0,0 +1,7 @@
+(module
+  (import "env" "func" (func $import (result i64)))
+  (func $defined (result i64)
+    (i64.add (i64.const 1) (i64.const 2))
+  )
+)
+

test/passes/log-execution.txt

@@ -1,21 +1,22 @@
 (module
- (type $0 (func))
+ (type $FUNCSIG$v (func))
  (type $1 (func (result i32)))
  (type $FUNCSIG$vi (func (param i32)))
+ (import "env" "func" (func $import))
  (import "env" "log_execution" (func $log_execution (param i32)))
- (func $nopp (; 1 ;) (type $0)
+ (func $nopp (; 2 ;) (type $FUNCSIG$v)
   (call $log_execution
    (i32.const 0)
   )
   (nop)
  )
- (func $intt (; 2 ;) (type $1) (result i32)
+ (func $intt (; 3 ;) (type $1) (result i32)
   (call $log_execution
    (i32.const 1)
   )
   (i32.const 10)
  )
- (func $workk (; 3 ;) (type $0)
+ (func $workk (; 4 ;) (type $FUNCSIG$v)
   (call $log_execution
    (i32.const 2)
   )
@@ -29,7 +30,7 @@
    )
   )
  )
- (func $loops (; 4 ;) (type $0)
+ (func $loops (; 5 ;) (type $FUNCSIG$v)
   (call $log_execution
    (i32.const 6)
   )
@@ -70,7 +71,7 @@
    )
   )
  )
- (func $loops-similar (; 5 ;) (type $0)
+ (func $loops-similar (; 6 ;) (type $FUNCSIG$v)
   (call $log_execution
    (i32.const 8)
   )

test/passes/log-execution.wast

@@ -1,4 +1,5 @@
 (module
+  (import "env" "func" (func $import))
   (func $nopp
     (nop)
   )

test/reduce/imports.wast

@@ -0,0 +1,17 @@
+(module
+  (import "env" "func" (func $import))
+  (export "x" (func $x))
+  (func $x (result i32)
+    (nop)
+    (nop)
+    (nop)
+    (call $import)
+    (drop (i32.const 1234))
+    (i32.const 5678) ;; easily reducible
+  )
+  (func $not-exported
+    (nop)
+    (unreachable)
+  )
+)
+

test/reduce/imports.wast.txt

@@ -0,0 +1,6 @@
+(module
+ (type $0 (func))
+ (type $1 (func (result i32)))
+ (import "env" "func" (func $fimport$0))
+)
+