diff --git a/src/passes/SimplifyGlobals.cpp b/src/passes/SimplifyGlobals.cpp index 84cf29be89f..4cd44b195e1 100644 --- a/src/passes/SimplifyGlobals.cpp +++ b/src/passes/SimplifyGlobals.cpp @@ -170,12 +170,42 @@ struct GlobalUseScanner : public WalkerPass> { if (codeEffects.hasAnything()) { return Name(); } + // Verify that we actually have a global.set here. We could also have a + // call to a function for whom we have computed function effects, but that + // is not what we want: such a function can be called from other places too. + // This read-only-to-write pattern must contain an actual global.set, so we + // can count the sets in the entire program to confirm that no dangerous + // ones exist. (In other words, we cannot take the shortcut of assuming that + // the effect "writes global $foo" means we actually have a global.set $foo + // here.) + auto found = false; + for (auto* set : FindAll(code).list) { + if (set->name == writtenGlobal) { + found = true; + break; + } + } + if (!found) { + return Name(); + } // See if we read that global in the condition expression. EffectAnalyzer conditionEffects(getPassOptions(), *getModule(), condition); if (!conditionEffects.mutableGlobalsRead.count(writtenGlobal)) { return Name(); } + // As above, confirm we see an actual global.get, and not a call to one with + // computed effects. + found = false; + for (auto* get : FindAll(condition).list) { + if (get->name == writtenGlobal) { + found = true; + break; + } + } + if (!found) { + return Name(); + } // If the condition has no other (non-removable) effects other than reading // that global then we have found what we looked for. diff --git a/test/lit/passes/simplify-globals_func-effects.wast b/test/lit/passes/simplify-globals_func-effects.wast index 4f088dd6153..d76eeda03ba 100644 --- a/test/lit/passes/simplify-globals_func-effects.wast +++ b/test/lit/passes/simplify-globals_func-effects.wast @@ -89,3 +89,328 @@ ) ) ) + +;; We do not optimize here, as while we have a read-only-to-write global and +;; function, there is another get and set. This setup will be used below to +;; test for effects in calls to those other functions. Note that it is clear we +;; do not optimize by the fact that the global remains mutable and the gets and +;; sets are not modified. +(module + ;; CHECK: (type $0 (func)) + + ;; CHECK: (type $1 (func (result i32))) + + ;; CHECK: (global $global (mut i32) (i32.const 0)) + (global $global (mut i32) (i32.const 0)) + + ;; CHECK: (export "read-only-to-write" (func $read-only-to-write)) + + ;; CHECK: (export "set" (func $set)) + + ;; CHECK: (export "get" (func $get)) + + ;; CHECK: (func $read-only-to-write + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (i32.eqz + ;; CHECK-NEXT: (global.get $global) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (global.set $global + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $read-only-to-write (export "read-only-to-write") + (if + (i32.eqz + (global.get $global) + ) + (then + (global.set $global + (i32.const 1) + ) + ) + ) + ) + + ;; CHECK: (func $set + ;; CHECK-NEXT: (global.set $global + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $set (export "set") + (global.set $global + (i32.const 1) + ) + ) + + ;; CHECK: (func $get (result i32) + ;; CHECK-NEXT: (global.get $global) + ;; CHECK-NEXT: ) + (func $get (export "get") (result i32) + (global.get $global) + ) +) + +(module + ;; CHECK: (type $0 (func)) + + ;; CHECK: (type $1 (func (result i32))) + + ;; CHECK: (global $global (mut i32) (i32.const 0)) + (global $global (mut i32) (i32.const 0)) + + ;; CHECK: (export "read-only-to-write" (func $read-only-to-write)) + + ;; CHECK: (export "set" (func $set)) + + ;; CHECK: (export "get" (func $get)) + + ;; CHECK: (func $read-only-to-write + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (block (result i32) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (call $get) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (global.set $global + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $read-only-to-write (export "read-only-to-write") + ;; This looks like it reads the global only to write it: we read the global + ;; through a call, then write it. However, that call is not an actual + ;; global.get, so we must not miscount here. There is only one global.get in + ;; the program, but it is *not* here, rather it is in a place it could be + ;; read from, separately (indeed, it is an export), so we *cannot* optimize + ;; the global as a read-only-to-write global: it has a read we do not + ;; control, which will in fact contain 1 after the "set" global is called, so + ;; we should not turn it into a constant 0 (and the global should remain + ;; mutable). + (if + (block (result i32) + (drop + (call $get) + ) + (i32.const 1) + ) + (then + (global.set $global + (i32.const 0) + ) + ) + ) + ) + + ;; CHECK: (func $set + ;; CHECK-NEXT: (global.set $global + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $set (export "set") + (global.set $global + (i32.const 1) + ) + ) + + ;; CHECK: (func $get (result i32) + ;; CHECK-NEXT: (global.get $global) + ;; CHECK-NEXT: ) + (func $get (export "get") (result i32) + (global.get $global) + ) +) + +;; As above, but now we call out to set the global, rather than get. Again, we +;; do not optimize. +(module + ;; CHECK: (type $0 (func)) + + ;; CHECK: (type $1 (func (result i32))) + + ;; CHECK: (global $global (mut i32) (i32.const 0)) + (global $global (mut i32) (i32.const 0)) + + ;; CHECK: (export "read-only-to-write" (func $read-only-to-write)) + + ;; CHECK: (export "set" (func $set)) + + ;; CHECK: (export "get" (func $get)) + + ;; CHECK: (func $read-only-to-write + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (i32.eqz + ;; CHECK-NEXT: (global.get $global) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (call $set) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $read-only-to-write (export "read-only-to-write") + (if + (i32.eqz + (global.get $global) + ) + (then + (call $set) + ) + ) + ) + + ;; CHECK: (func $set + ;; CHECK-NEXT: (global.set $global + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $set (export "set") + (global.set $global + (i32.const 1) + ) + ) + + ;; CHECK: (func $get (result i32) + ;; CHECK-NEXT: (global.get $global) + ;; CHECK-NEXT: ) + (func $get (export "get") (result i32) + (global.get $global) + ) +) + +;; As above, but now we call out to set and get. Again, we do not optimize. +(module + ;; CHECK: (type $0 (func)) + + ;; CHECK: (type $1 (func (result i32))) + + ;; CHECK: (global $global (mut i32) (i32.const 0)) + (global $global (mut i32) (i32.const 0)) + + ;; CHECK: (export "read-only-to-write" (func $read-only-to-write)) + + ;; CHECK: (export "set" (func $set)) + + ;; CHECK: (export "get" (func $get)) + + ;; CHECK: (func $read-only-to-write + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (block (result i32) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (call $get) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (call $set) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $read-only-to-write (export "read-only-to-write") + (if + (block (result i32) + (drop + (call $get) + ) + (i32.const 1) + ) + (then + (call $set) + ) + ) + ) + + ;; CHECK: (func $set + ;; CHECK-NEXT: (global.set $global + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $set (export "set") + (global.set $global + (i32.const 1) + ) + ) + + ;; CHECK: (func $get (result i32) + ;; CHECK-NEXT: (global.get $global) + ;; CHECK-NEXT: ) + (func $get (export "get") (result i32) + (global.get $global) + ) +) + +;; As above, but with a second global. +(module + ;; CHECK: (type $0 (func)) + + ;; CHECK: (type $1 (func (result i32))) + + ;; CHECK: (global $global (mut i32) (i32.const 0)) + (global $global (mut i32) (i32.const 0)) + + ;; CHECK: (global $other i32 (i32.const 0)) + (global $other (mut i32) (i32.const 0)) + + ;; CHECK: (export "read-only-to-write" (func $read-only-to-write)) + + ;; CHECK: (export "set" (func $set)) + + ;; CHECK: (export "get" (func $get)) + + ;; CHECK: (func $read-only-to-write + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (block (result i32) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (call $get) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (global.set $global + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $read-only-to-write (export "read-only-to-write") + ;; We *do* have a global.get here, but it is of the wrong global, $other, so + ;; we should not optimize $global (we can optimize $other to be immutable, + ;; though, and apply its value of 0 here). + (if + (block (result i32) + (drop + (call $get) + ) + (global.get $other) + ) + (then + (global.set $global + (i32.const 0) + ) + ) + ) + ) + + ;; CHECK: (func $set + ;; CHECK-NEXT: (global.set $global + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $set (export "set") + (global.set $global + (i32.const 1) + ) + ) + + ;; CHECK: (func $get (result i32) + ;; CHECK-NEXT: (global.get $global) + ;; CHECK-NEXT: ) + (func $get (export "get") (result i32) + (global.get $global) + ) +) +