GlobalTypeOptimization: Handle side effects in removed fields in struct.new (#4263)

If struct.new operands have side effects, and we are removing the operand
as the field is removed, we must keep the side effects. To handle that, store
all the operands in locals and read from the locals, and then removing a
local.get is always safe to do, and nothing has been reordered:

(struct.new
  (A)
  (side effect) ;; this field will be removed
  (B)
)

=>

(local.set $a (A))
(local.set $t (side effect))
(local.set $b (B))
(struct.new
  (local.get $a)
  (local.get $b)
)

Later passes can remove unneeded local operations etc.

This is necessary before enabling this pass, as this corner case occurs on
j2wasm.

2 files changed, 77 insertions, 8 deletions

diff --git a/src/ir/localize.h b/src/ir/localize.h
index c8e85822a..44ec5af32 100644
--- a/src/ir/localize.h
+++ b/src/ir/localize.h
@@ -17,7 +17,8 @@
 #ifndef wasm_ir_localizer_h
 #define wasm_ir_localizer_h
 
-#include <wasm-builder.h>
+#include "ir/iteration.h"
+#include "wasm-builder.h"
 
 namespace wasm {
 
@@ -26,7 +27,6 @@ namespace wasm {
 //
 // Note that if the local is reused, this assumes it is not modified in between
 // the set and the get, which the caller must ensure.
-
 struct Localizer {
   Index index;
   Expression* expr;
@@ -44,6 +44,44 @@ struct Localizer {
   }
 };
 
+// Replaces all children with gets of locals, if they have any effects. After
+// this, the original input has only local.gets as inputs, or other things that
+// have no interacting effects, and so those children can be reordered.
+// The sets of the locals are emitted on a |sets| property on the class. Those
+// must be emitted right before the input.
+// This stops at the first unreachable child, as there is no code executing
+// after that point anyhow.
+// TODO: use in more places
+struct ChildLocalizer {
+  std::vector<LocalSet*> sets;
+
+  ChildLocalizer(Expression* input,
+                 Function* func,
+                 Module* wasm,
+                 const PassOptions& options) {
+    Builder builder(*wasm);
+    ChildIterator iterator(input);
+    auto& children = iterator.children;
+    // The children are in reverse order, so allocate the output first and
+    // apply items as we go.
+    auto num = children.size();
+    for (Index i = 0; i < num; i++) {
+      auto** childp = children[num - 1 - i];
+      auto* child = *childp;
+      if (child->type == Type::unreachable) {
+        break;
+      }
+      // If there are effects, use a local for this.
+      // TODO: Compare interactions between their side effects.
+      if (EffectAnalyzer(options, *wasm, child).hasAnything()) {
+        auto local = builder.addVar(func, child->type);
+        sets.push_back(builder.makeLocalSet(local, child));
+        *childp = builder.makeLocalGet(local, child->type);
+      }
+    }
+  }
+};
+
 } // namespace wasm
 
 #endif // wasm_ir_localizer_h
diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp
index 442b7570a..7ca7c4f85 100644
--- a/src/passes/GlobalTypeOptimization.cpp
+++ b/src/passes/GlobalTypeOptimization.cpp
@@ -22,10 +22,10 @@
 //  * Fields that are never read from can be removed entirely.
 //
 // TODO: Specialize field types.
-// TODO: Remove unused fields.
 //
 
 #include "ir/effects.h"
+#include "ir/localize.h"
 #include "ir/struct-utils.h"
 #include "ir/subtypes.h"
 #include "ir/type-updating.h"
@@ -300,6 +300,33 @@ struct GlobalTypeOptimization : public Pass {
         auto& operands = curr->operands;
         assert(indexesAfterRemoval.size() == operands.size());
 
+        // Check for side effects in removed fields. If there are any, we must
+        // use locals to save the values (while keeping them in order).
+        bool useLocals = false;
+        for (Index i = 0; i < operands.size(); i++) {
+          auto newIndex = indexesAfterRemoval[i];
+          if (newIndex == RemovedField &&
+              EffectAnalyzer(getPassOptions(), *getModule(), operands[i])
+                .hasUnremovableSideEffects()) {
+            useLocals = true;
+            break;
+          }
+        }
+        if (useLocals) {
+          auto* func = getFunction();
+          if (!func) {
+            Fatal() << "TODO: side effects in removed fields in globals\n";
+          }
+          auto* block = Builder(*getModule()).makeBlock();
+          auto sets =
+            ChildLocalizer(curr, func, getModule(), getPassOptions()).sets;
+          block->list.set(sets);
+          block->list.push_back(curr);
+          block->finalize(curr->type);
+          replaceCurrent(block);
+          addedLocals = true;
+        }
+
         // Remove the unneeded operands.
         Index removed = 0;
         for (Index i = 0; i < operands.size(); i++) {
@@ -308,11 +335,6 @@ struct GlobalTypeOptimization : public Pass {
             assert(newIndex < operands.size());
             operands[newIndex] = operands[i];
           } else {
-            if (EffectAnalyzer(getPassOptions(), *getModule(), operands[i])
-                  .hasUnremovableSideEffects()) {
-              Fatal() << "TODO: handle side effects in field removal "
-                         "(impossible in global locations?)";
-            }
             removed++;
           }
         }
@@ -347,6 +369,15 @@ struct GlobalTypeOptimization : public Pass {
         curr->index = newIndex;
       }
 
+      void visitFunction(Function* curr) {
+        if (addedLocals) {
+          TypeUpdating::handleNonDefaultableLocals(curr, *getModule());
+        }
+      }
+
+    private:
+      bool addedLocals = false;
+
       Index getNewIndex(HeapType type, Index index) {
         auto iter = parent.indexesAfterRemovals.find(type);
         if (iter == parent.indexesAfterRemovals.end()) {