diff --git a/src/passes/Precompute.cpp b/src/passes/Precompute.cpp index 89e618ab3e1..5baf2331f14 100644 --- a/src/passes/Precompute.cpp +++ b/src/passes/Precompute.cpp @@ -799,11 +799,9 @@ struct Precompute if (type.isFunction()) { return true; } - // We can emit a StringConst for a string constant in principle, but we do - // not want to as that might cause more allocations to happen. See similar - // code in SimplifyGlobals. + // We can emit a StringConst for a string constant. if (type.isString()) { - return false; + return true; } // All other reference types cannot be precomputed. Even an immutable GC // reference is not currently something this pass can handle, as it will diff --git a/src/passes/SimplifyGlobals.cpp b/src/passes/SimplifyGlobals.cpp index a7d8c348746..3baec440931 100644 --- a/src/passes/SimplifyGlobals.cpp +++ b/src/passes/SimplifyGlobals.cpp @@ -52,20 +52,6 @@ namespace wasm { namespace { -// Checks if an expression is a constant, and one that we can copy without -// downsides. This is the set of constant values that we will inline from -// globals. -bool isCopyableConstant(Expression* curr) { - // Anything that is truly constant is suitable for us, *except* for string - // constants, which in VMs may be implemented not as a constant but as an - // allocation. We prefer to keep string.const in globals where any such - // allocation only happens once (note that that makes them equivalent to - // strings imported from JS, which would be in imported globals, which are - // similarly not optimizable). - // TODO: revisit this if/when VMs implement and optimize string.const. - return Properties::isConstantExpression(curr) && !curr->is(); -} - struct GlobalInfo { // Whether the global is imported and exported. bool imported = false; @@ -372,7 +358,7 @@ struct ConstantGlobalApplier void visitExpression(Expression* curr) { if (auto* set = curr->dynCast()) { - if (isCopyableConstant(set->value)) { + if (Properties::isConstantExpression(set->value)) { currConstantGlobals[set->name] = getLiteralsFromConstExpression(set->value); } else { @@ -383,7 +369,7 @@ struct ConstantGlobalApplier // Check if the global is known to be constant all the time. if (constantGlobals->count(get->name)) { auto* global = getModule()->getGlobal(get->name); - assert(isCopyableConstant(global->init)); + assert(Properties::isConstantExpression(global->init)); replaceCurrent(ExpressionManipulator::copy(global->init, *getModule())); replaced = true; return; @@ -685,7 +671,7 @@ struct SimplifyGlobals : public Pass { // go, as well as applying them where possible. for (auto& global : module->globals) { if (!global->imported()) { - if (isCopyableConstant(global->init)) { + if (Properties::isConstantExpression(global->init)) { constantGlobals[global->name] = getLiteralsFromConstExpression(global->init); } else { @@ -709,7 +695,7 @@ struct SimplifyGlobals : public Pass { NameSet constantGlobals; for (auto& global : module->globals) { if (!global->mutable_ && !global->imported() && - isCopyableConstant(global->init)) { + Properties::isConstantExpression(global->init)) { constantGlobals.insert(global->name); } } diff --git a/test/lit/passes/precompute-gc.wast b/test/lit/passes/precompute-gc.wast index 603788f8c82..8d4b56ce760 100644 --- a/test/lit/passes/precompute-gc.wast +++ b/test/lit/passes/precompute-gc.wast @@ -1021,10 +1021,10 @@ ;; CHECK-NEXT: (string.const "hello, world") ;; CHECK-NEXT: ) ;; CHECK-NEXT: (call $strings - ;; CHECK-NEXT: (local.get $s) + ;; CHECK-NEXT: (string.const "hello, world") ;; CHECK-NEXT: ) ;; CHECK-NEXT: (call $strings - ;; CHECK-NEXT: (local.get $s) + ;; CHECK-NEXT: (string.const "hello, world") ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $strings (param $param (ref string)) @@ -1032,9 +1032,7 @@ (local.set $s (string.const "hello, world") ) - ;; The constant string could be propagated twice, to both of these calls, but - ;; we do not do so as it might cause more allocations to happen. - ;; TODO if VMs optimize that, handle it + ;; The constant string should be propagated twice, to both of these calls. (call $strings (local.get $s) ) diff --git a/test/lit/passes/simplify-globals-strings.wast b/test/lit/passes/simplify-globals-strings.wast deleted file mode 100644 index 813e2964ce4..00000000000 --- a/test/lit/passes/simplify-globals-strings.wast +++ /dev/null @@ -1,65 +0,0 @@ -;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. -;; NOTE: This test was ported using port_passes_tests_to_lit.py and could be cleaned up. - -;; We do not "inline" strings from globals, as that might cause more -;; allocations to happen. TODO if VMs optimize that, remove this - -;; RUN: foreach %s %t wasm-opt --simplify-globals -all -S -o - | filecheck %s - -;; Also test with -O3 --gufa to see that no other pass does this kind of thing, -;; as extra coverage. - -;; RUN: foreach %s %t wasm-opt -O3 --gufa -all -S -o - | filecheck %s --check-prefix=O3GUF - -(module - ;; CHECK: (type $0 (func (result anyref))) - - ;; CHECK: (global $string stringref (string.const "one")) - ;; O3GUF: (type $0 (func (result anyref))) - - ;; O3GUF: (global $string (ref string) (string.const "one")) - (global $string stringref (string.const "one")) - - ;; CHECK: (global $string-mut (mut stringref) (string.const "two")) - ;; O3GUF: (global $string-mut (mut (ref string)) (string.const "two")) - (global $string-mut (mut stringref) (string.const "two")) - - ;; CHECK: (export "global" (func $global)) - ;; O3GUF: (export "global" (func $global)) - (export "global" (func $global)) - - ;; CHECK: (export "written" (func $written)) - ;; O3GUF: (export "written" (func $written)) - (export "written" (func $written)) - - ;; CHECK: (func $global (type $0) (result anyref) - ;; CHECK-NEXT: (global.get $string) - ;; CHECK-NEXT: ) - ;; O3GUF: (func $global (type $0) (result anyref) - ;; O3GUF-NEXT: (global.get $string) - ;; O3GUF-NEXT: ) - (func $global (result anyref) - ;; This should not turn into "one". - (global.get $string) - ) - - ;; CHECK: (func $written (type $0) (result anyref) - ;; CHECK-NEXT: (global.set $string-mut - ;; CHECK-NEXT: (string.const "three") - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (global.get $string-mut) - ;; CHECK-NEXT: ) - ;; O3GUF: (func $written (type $0) (result anyref) - ;; O3GUF-NEXT: (global.set $string-mut - ;; O3GUF-NEXT: (string.const "three") - ;; O3GUF-NEXT: ) - ;; O3GUF-NEXT: (global.get $string-mut) - ;; O3GUF-NEXT: ) - (func $written (result anyref) - (global.set $string-mut - (string.const "three") - ) - ;; This should not turn into "three". - (global.get $string-mut) - ) -)