From 2a4c278d19497aae84c42f5f3f18a1fed0222603 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 31 Oct 2023 12:48:13 -0700 Subject: [PATCH] Followup to #6061 --- src/passes/OnceReduction.cpp | 22 +++++++++++++--------- test/lit/passes/once-reduction.wast | 12 +++++------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/passes/OnceReduction.cpp b/src/passes/OnceReduction.cpp index d97e19302dc..936c590eeb4 100644 --- a/src/passes/OnceReduction.cpp +++ b/src/passes/OnceReduction.cpp @@ -486,14 +486,16 @@ struct OnceReduction : public Pass { ExpressionManipulator::nop(body); continue; } - if (list.size() != 3) { - // Something non-trivial; too many items for us to consider. - continue; - } - auto* payload = list[2]; - if (auto* call = payload->dynCast()) { + // The early-exit logic is the first item, followed by the global setting, + // and we've ruled out the case of there being nothing else after those, + // so there are at least 3 items. + assert(list.size() >= 3); + // We consider the first item in the payload. Anything further would be + // much more difficult to analyze. + auto* payloadStart = list[2]; + if (auto* call = payloadStart->dynCast()) { if (optInfo.onceFuncs.at(call->target).is()) { - // All this "once" function does is call another. We do not need the + // This "once" function immediately calls another. We do not need the // early-exit logic in this one, then, because of the following // reasoning. We are comparing these forms: // @@ -502,6 +504,7 @@ struct OnceReduction : public Pass { // if (!foo$once) return; // two lines of // foo$once = 1; // early-exit code // bar(); + // .. // } // // to @@ -509,12 +512,13 @@ struct OnceReduction : public Pass { // // AFTER // function foo() { // bar(); + // .. // } // // The question is whether different behavior can be observed between // those two. There are two cases, when we enter foo: // - // 1. foo has been called before. Then we early-exit in BEFORE, and + // 1. foo has been entered before. Then we early-exit in BEFORE, and // in AFTER we call bar which will early-exit (since foo was // called, which means bar was at least entered, which set its // global; bar might be on the stack, if it called foo, so it has @@ -522,7 +526,7 @@ struct OnceReduction : public Pass { // handle in general, like recursive imports of modules in various // languages - but we do know bar has been *entered*, which means // the global was set). - // 2. foo has never been called before. In this case in BEFORE we set + // 2. foo has never been entered before. In this case in BEFORE we set // the global and call bar, and in AFTER we also call bar. // // Thus, the behavior is the same, and we can remove the early-exit diff --git a/test/lit/passes/once-reduction.wast b/test/lit/passes/once-reduction.wast index 45170a0e185..aab873261ac 100644 --- a/test/lit/passes/once-reduction.wast +++ b/test/lit/passes/once-reduction.wast @@ -1591,13 +1591,8 @@ ;; CHECK: (func $once (type $0) - ;; CHECK-NEXT: (if - ;; CHECK-NEXT: (global.get $once) - ;; CHECK-NEXT: (return) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (global.set $once - ;; CHECK-NEXT: (i32.const 1) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: (call $once.1) ;; CHECK-NEXT: (call $once.2) ;; CHECK-NEXT: (call $import @@ -1610,6 +1605,9 @@ (return) ) (global.set $once (i32.const 1)) + ;; We immediately call another "once" function, so we can remove the early- + ;; exit logic before us. (Note that $once.1 and $once.2 call us, but there + ;; we cannot remove anything because of the risk of infinite looping.) (call $once.1) ;; We cannot remove this second call. While $once.1 calls $once.2, we may ;; be in this situation: a call started at $once.1, which calls $once