wasm-merge: Sort globals to ensure proper validation

src/passes/ReorderGlobals.cpp

@@ -58,7 +58,11 @@ struct UseCountScanner : public WalkerPass<PostWalker<UseCountScanner>> {
 
 struct ReorderGlobals : public Pass {
   // Whether to always reorder globals, even if there are very few and the
-  // benefit is minor. That is useful for testing.
+  // benefit is minor. That is useful for testing, and also internally in passes
+  // that use us to reorder them so dependencies appear first (that is, if a
+  // pass ends up with an earlier global reading a later one, the sorting in
+  // this pass will reorder them properly; we need to take those dependencies
+  // into account anyhow here).
   bool always;
 
   ReorderGlobals(bool always) : always(always) {}

src/tools/wasm-merge.cpp

@@ -586,13 +586,17 @@ Note that filenames and modules names are interleaved (which is hopefully less c
   // module.
   fuseImportsAndExports();
 
-  // Remove unused things. This is obviously a useful optimization, but it also
-  // makes using the output easier: if an import was resolved by an export
-  // during the merge, then that import will have no more uses and it will be
-  // optimized out (while if we didn't optimize it out then instantiating the
-  // module would still be forced to provide something for that import).
   {
     PassRunner passRunner(&merged);
+    // We might have made some globals read from others that now appear after
+    // them (if the one they read was appended from a later module). Sort them
+    // to fix that. TODO: we could do this only if we actually append globals
+    passRunner.add("reorder-globals-always");
+    // Remove unused things. This is obviously a useful optimization but it also
+    // makes using the output easier: if an import was resolved by an export
+    // during the merge, then that import will have no more uses and it will be
+    // optimized out (while if we didn't optimize it out then instantiating the
+    // module would still be forced to provide something for that import).
     passRunner.add("remove-unused-module-elements");
     passRunner.run();
   }

test/lit/merge/global-ordering.wat

@@ -0,0 +1,30 @@
+;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
+
+;; RUN: wasm-merge %s first %s.second second -all -S -o - | filecheck %s
+
+;; After the merge this module's global will read a value from a global that is
+;; appended after it, from the second module. Those must be reordered so that
+;; we validate, as a global can only read from previous ones.
+
+(module
+  (import "second" "second.global.export" (global i32))
+
+  ;; CHECK:      (type $0 (func (result i32)))
+
+  ;; CHECK:      (global $second.global i32 (i32.const 42))
+
+  ;; CHECK:      (global $first.global (mut i32) (global.get $second.global))
+  (global $first.global (mut i32) (global.get 0))
+
+  ;; CHECK:      (export "run" (func $run))
+
+  ;; CHECK:      (export "second.global.export" (global $second.global))
+
+  ;; CHECK:      (func $run (type $0) (result i32)
+  ;; CHECK-NEXT:  (global.get $first.global)
+  ;; CHECK-NEXT: )
+  (func $run (export "run") (result i32)
+    ;; Use the global to avoid it being removed.
+    (global.get $first.global)
+  )
+)

test/lit/merge/global-ordering.wat.second

@@ -0,0 +1,3 @@
+(module
+  (global $second.global (export "second.global.export") i32 (i32.const 42))
+)

test/lit/merge/renamings.wat

@@ -23,20 +23,21 @@
 
   ;; CHECK:      (import "elsewhere" "some.tag" (tag $imported (param f64)))
 
+  ;; CHECK:      (global $bar_2 i32 (i32.const 4))
+
+  ;; CHECK:      (global $other i32 (i32.const 3))
+
+  ;; CHECK:      (global $bar i32 (i32.const 2))
+
   ;; CHECK:      (global $foo i32 (i32.const 1))
   (global $foo i32 (i32.const 1))
 
   ;; This global has a conflict in second.wat, and so second.wat's $bar
   ;; will be renamed.
-  ;; CHECK:      (global $bar i32 (i32.const 2))
   (global $bar i32 (i32.const 2))
 
   ;; This memory has a conflict in second.wat, and so second.wat's $foo
   ;; will be renamed.
-  ;; CHECK:      (global $other i32 (i32.const 3))
-
-  ;; CHECK:      (global $bar_2 i32 (i32.const 4))
-
   ;; CHECK:      (memory $foo 10 20)
   (memory $foo 10 20)