From 9241cb7dca8c15e7e3a875c08ac72a4d4a11df05 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 9 Mar 2026 12:26:00 -0700 Subject: [PATCH 1/5] go --- src/passes/SimplifyGlobals.cpp | 17 +- .../passes/simplify-globals_func-effects.wast | 254 ++++++++++++++++++ 2 files changed, 270 insertions(+), 1 deletion(-) diff --git a/src/passes/SimplifyGlobals.cpp b/src/passes/SimplifyGlobals.cpp index 84cf29be89f..d9b12b07f6a 100644 --- a/src/passes/SimplifyGlobals.cpp +++ b/src/passes/SimplifyGlobals.cpp @@ -170,18 +170,31 @@ 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. + if (FindAll(code).list.empty()) { + return; + } // See if we read that global in the condition expression. EffectAnalyzer conditionEffects(getPassOptions(), *getModule(), condition); if (!conditionEffects.mutableGlobalsRead.count(writtenGlobal)) { return Name(); } - // If the condition has no other (non-removable) effects other than reading // that global then we have found what we looked for. if (!conditionEffects.hasUnremovableSideEffects()) { return writtenGlobal; } + // As above, confirm we see an actual global.get, and not a call to one with + // computed effects. + if (FindAll(condition).list.empty()) { + return; + } // There are unremovable side effects of some form. However, they may not // be related to the reading of the global, that is, the global's value may @@ -578,6 +591,8 @@ struct SimplifyGlobals : public Pass { if (!info.read || !info.nonInitWritten || onlyReadOnlyToWrite) { globalsNotNeedingSets.insert(global->name); +std::cout << "no need " << global->name << " because read=" << info.read << " : " << info.nonInitWritten << " : " << onlyReadOnlyToWrite << '\n'; +std::cout << " info.readOnlyToWrite=" << info.readOnlyToWrite << '\n'; // We can now mark this global as immutable, and un-written, since we // are about to remove all the sets on it. diff --git a/test/lit/passes/simplify-globals_func-effects.wast b/test/lit/passes/simplify-globals_func-effects.wast index 4f088dd6153..2c50ba93324 100644 --- a/test/lit/passes/simplify-globals_func-effects.wast +++ b/test/lit/passes/simplify-globals_func-effects.wast @@ -89,3 +89,257 @@ ) ) ) + +;; 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 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: (drop + ;; 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: (drop + ;; 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: (i32.const 0) + ;; 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 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: (drop + ;; 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: (i32.const 0) + ;; CHECK-NEXT: ) + (func $get (export "get") (result i32) + (global.get $global) + ) +) + From 1d0859d01d8f1f08aec13f78f0631ac180771155 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 9 Mar 2026 12:26:58 -0700 Subject: [PATCH 2/5] fix --- src/passes/SimplifyGlobals.cpp | 4 ++-- test/lit/passes/simplify-globals_func-effects.wast | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/passes/SimplifyGlobals.cpp b/src/passes/SimplifyGlobals.cpp index d9b12b07f6a..44a9779ea1f 100644 --- a/src/passes/SimplifyGlobals.cpp +++ b/src/passes/SimplifyGlobals.cpp @@ -177,7 +177,7 @@ struct GlobalUseScanner : public WalkerPass> { // can count the sets in the entire program to confirm that no dangerous // ones exist. if (FindAll(code).list.empty()) { - return; + return Name(); } // See if we read that global in the condition expression. @@ -193,7 +193,7 @@ struct GlobalUseScanner : public WalkerPass> { // As above, confirm we see an actual global.get, and not a call to one with // computed effects. if (FindAll(condition).list.empty()) { - return; + return Name(); } // There are unremovable side effects of some form. However, they may not diff --git a/test/lit/passes/simplify-globals_func-effects.wast b/test/lit/passes/simplify-globals_func-effects.wast index 2c50ba93324..479db2c450b 100644 --- a/test/lit/passes/simplify-globals_func-effects.wast +++ b/test/lit/passes/simplify-globals_func-effects.wast @@ -288,7 +288,7 @@ ;; CHECK: (type $1 (func (result i32))) - ;; CHECK: (global $global i32 (i32.const 0)) + ;; 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)) @@ -325,7 +325,7 @@ ) ;; CHECK: (func $set - ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (global.set $global ;; CHECK-NEXT: (i32.const 1) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) @@ -336,7 +336,7 @@ ) ;; CHECK: (func $get (result i32) - ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: (global.get $global) ;; CHECK-NEXT: ) (func $get (export "get") (result i32) (global.get $global) From f44566ce6c6b0f3c8b9b6851770223913f00eac7 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 9 Mar 2026 12:30:15 -0700 Subject: [PATCH 3/5] fix --- src/passes/SimplifyGlobals.cpp | 15 +++++++++------ .../lit/passes/simplify-globals_func-effects.wast | 8 ++++---- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/passes/SimplifyGlobals.cpp b/src/passes/SimplifyGlobals.cpp index 44a9779ea1f..b225e1c1002 100644 --- a/src/passes/SimplifyGlobals.cpp +++ b/src/passes/SimplifyGlobals.cpp @@ -175,7 +175,9 @@ struct GlobalUseScanner : public WalkerPass> { // 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. + // 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.) if (FindAll(code).list.empty()) { return Name(); } @@ -185,17 +187,18 @@ struct GlobalUseScanner : public WalkerPass> { if (!conditionEffects.mutableGlobalsRead.count(writtenGlobal)) { return Name(); } - // If the condition has no other (non-removable) effects other than reading - // that global then we have found what we looked for. - if (!conditionEffects.hasUnremovableSideEffects()) { - return writtenGlobal; - } // As above, confirm we see an actual global.get, and not a call to one with // computed effects. if (FindAll(condition).list.empty()) { return Name(); } + // If the condition has no other (non-removable) effects other than reading + // that global then we have found what we looked for. + if (!conditionEffects.hasUnremovableSideEffects()) { + return writtenGlobal; + } + // There are unremovable side effects of some form. However, they may not // be related to the reading of the global, that is, the global's value may // not flow to anything that uses it in a dangerous way. It *would* be diff --git a/test/lit/passes/simplify-globals_func-effects.wast b/test/lit/passes/simplify-globals_func-effects.wast index 479db2c450b..52ca557774c 100644 --- a/test/lit/passes/simplify-globals_func-effects.wast +++ b/test/lit/passes/simplify-globals_func-effects.wast @@ -158,7 +158,7 @@ ;; CHECK: (type $1 (func (result i32))) - ;; CHECK: (global $global i32 (i32.const 0)) + ;; 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)) @@ -176,7 +176,7 @@ ;; CHECK-NEXT: (i32.const 1) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (then - ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (global.set $global ;; CHECK-NEXT: (i32.const 0) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) @@ -208,7 +208,7 @@ ) ;; CHECK: (func $set - ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (global.set $global ;; CHECK-NEXT: (i32.const 1) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) @@ -219,7 +219,7 @@ ) ;; CHECK: (func $get (result i32) - ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: (global.get $global) ;; CHECK-NEXT: ) (func $get (export "get") (result i32) (global.get $global) From c986728260ab6d6e04f71c76fcf2d66e360c363e Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 9 Mar 2026 12:30:27 -0700 Subject: [PATCH 4/5] clean --- src/passes/SimplifyGlobals.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/passes/SimplifyGlobals.cpp b/src/passes/SimplifyGlobals.cpp index b225e1c1002..650df994263 100644 --- a/src/passes/SimplifyGlobals.cpp +++ b/src/passes/SimplifyGlobals.cpp @@ -594,8 +594,6 @@ struct SimplifyGlobals : public Pass { if (!info.read || !info.nonInitWritten || onlyReadOnlyToWrite) { globalsNotNeedingSets.insert(global->name); -std::cout << "no need " << global->name << " because read=" << info.read << " : " << info.nonInitWritten << " : " << onlyReadOnlyToWrite << '\n'; -std::cout << " info.readOnlyToWrite=" << info.readOnlyToWrite << '\n'; // We can now mark this global as immutable, and un-written, since we // are about to remove all the sets on it. From 747f671d5a1dafe9f901757f44249ebfdf32c0a2 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 9 Mar 2026 13:39:41 -0700 Subject: [PATCH 5/5] fix --- src/passes/SimplifyGlobals.cpp | 18 ++++- .../passes/simplify-globals_func-effects.wast | 71 +++++++++++++++++++ 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/src/passes/SimplifyGlobals.cpp b/src/passes/SimplifyGlobals.cpp index 650df994263..4cd44b195e1 100644 --- a/src/passes/SimplifyGlobals.cpp +++ b/src/passes/SimplifyGlobals.cpp @@ -178,7 +178,14 @@ struct GlobalUseScanner : public WalkerPass> { // 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.) - if (FindAll(code).list.empty()) { + auto found = false; + for (auto* set : FindAll(code).list) { + if (set->name == writtenGlobal) { + found = true; + break; + } + } + if (!found) { return Name(); } @@ -189,7 +196,14 @@ struct GlobalUseScanner : public WalkerPass> { } // As above, confirm we see an actual global.get, and not a call to one with // computed effects. - if (FindAll(condition).list.empty()) { + found = false; + for (auto* get : FindAll(condition).list) { + if (get->name == writtenGlobal) { + found = true; + break; + } + } + if (!found) { return Name(); } diff --git a/test/lit/passes/simplify-globals_func-effects.wast b/test/lit/passes/simplify-globals_func-effects.wast index 52ca557774c..d76eeda03ba 100644 --- a/test/lit/passes/simplify-globals_func-effects.wast +++ b/test/lit/passes/simplify-globals_func-effects.wast @@ -343,3 +343,74 @@ ) ) +;; 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) + ) +) +