From 9090ce56fcc67e15005aeedc59c6bc6773220f11 Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Tue, 23 Jan 2024 17:30:38 -0800
Subject: Stop propagating/inlining string constants (#6234)

This causes overhead atm since in VMs executing a string.const will actually allocate a
string, and more copies means more allocations. For now, just do not add more. This
required changes to two passes: SimplifyGlobals and Precompute.
---
 src/passes/SimplifyGlobals.cpp | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

(limited to 'src/passes/SimplifyGlobals.cpp')

diff --git a/src/passes/SimplifyGlobals.cpp b/src/passes/SimplifyGlobals.cpp
index 3baec4409..a7d8c3487 100644
--- a/src/passes/SimplifyGlobals.cpp
+++ b/src/passes/SimplifyGlobals.cpp
@@ -52,6 +52,20 @@ 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<StringConst>();
+}
+
 struct GlobalInfo {
   // Whether the global is imported and exported.
   bool imported = false;
@@ -358,7 +372,7 @@ struct ConstantGlobalApplier
 
   void visitExpression(Expression* curr) {
     if (auto* set = curr->dynCast<GlobalSet>()) {
-      if (Properties::isConstantExpression(set->value)) {
+      if (isCopyableConstant(set->value)) {
         currConstantGlobals[set->name] =
           getLiteralsFromConstExpression(set->value);
       } else {
@@ -369,7 +383,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(Properties::isConstantExpression(global->init));
+        assert(isCopyableConstant(global->init));
         replaceCurrent(ExpressionManipulator::copy(global->init, *getModule()));
         replaced = true;
         return;
@@ -671,7 +685,7 @@ struct SimplifyGlobals : public Pass {
     // go, as well as applying them where possible.
     for (auto& global : module->globals) {
       if (!global->imported()) {
-        if (Properties::isConstantExpression(global->init)) {
+        if (isCopyableConstant(global->init)) {
           constantGlobals[global->name] =
             getLiteralsFromConstExpression(global->init);
         } else {
@@ -695,7 +709,7 @@ struct SimplifyGlobals : public Pass {
     NameSet constantGlobals;
     for (auto& global : module->globals) {
       if (!global->mutable_ && !global->imported() &&
-          Properties::isConstantExpression(global->init)) {
+          isCopyableConstant(global->init)) {
         constantGlobals.insert(global->name);
       }
     }
-- 
cgit v1.2.3