Revert "Remove bulk memory instructions refering to active segments (#2235)" (#2244)

This reverts commit 72c52ea7d4eb61b95cf8a5164947cb760fe42e9c, which was causing test failures after it merged.

3 files changed, 12 insertions, 59 deletions

diff --git a/src/ir/manipulation.h b/src/ir/manipulation.h
index ec137d372..d363fc547 100644
--- a/src/ir/manipulation.h
+++ b/src/ir/manipulation.h
@@ -38,11 +38,6 @@ template<typename InputType> inline Nop* nop(InputType* target) {
   return convert<InputType, Nop>(target);
 }
 
-template<typename InputType>
-inline Unreachable* unreachable(InputType* target) {
-  return convert<InputType, Unreachable>(target);
-}
-
 // Convert a node that allocates
 template<typename InputType, typename OutputType>
 inline OutputType* convert(InputType* input, MixedArena& allocator) {
diff --git a/src/passes/MemoryPacking.cpp b/src/passes/MemoryPacking.cpp
index f98c25096..0b24379e5 100644
--- a/src/passes/MemoryPacking.cpp
+++ b/src/passes/MemoryPacking.cpp
@@ -14,8 +14,6 @@
  * limitations under the License.
  */
 
-#include "ir/manipulation.h"
-#include "ir/utils.h"
 #include "pass.h"
 #include "wasm-binary.h"
 #include "wasm-builder.h"
@@ -34,13 +32,11 @@ struct MemoryPacking : public Pass {
       return;
     }
 
+    // Conservatively refuse to change segments if any are passive to avoid
+    // invalidating segment indices or segment contents referenced from
+    // memory.init instructions.
+    // TODO: optimize in the presence of memory.init instructions
     if (module->features.hasBulkMemory()) {
-      // Remove any references to active segments that might be invalidated.
-      optimizeTrappingBulkMemoryOps(runner, module);
-      // Conservatively refuse to change segments if any are passive to avoid
-      // invalidating segment indices or segment contents referenced from
-      // memory.init and data.drop instructions.
-      // TODO: optimize in the presence of memory.init and  data.drop
       for (auto segment : module->memory.segments) {
         if (segment.isPassive) {
           return;
@@ -124,38 +120,6 @@ struct MemoryPacking : public Pass {
     }
     module->memory.segments.swap(packed);
   }
-
-  void optimizeTrappingBulkMemoryOps(PassRunner* runner, Module* module) {
-    struct Trapper : WalkerPass<PostWalker<Trapper>> {
-      bool changed;
-      void visitMemoryInit(MemoryInit* curr) {
-        if (!getModule()->memory.segments[curr->segment].isPassive) {
-          Builder builder(*getModule());
-          replaceCurrent(builder.blockify(builder.makeDrop(curr->dest),
-                                          builder.makeDrop(curr->offset),
-                                          builder.makeDrop(curr->size),
-                                          builder.makeUnreachable()));
-          changed = true;
-        }
-      }
-      void visitDataDrop(DataDrop* curr) {
-        if (!getModule()->memory.segments[curr->segment].isPassive) {
-          ExpressionManipulator::unreachable(curr);
-          changed = true;
-        }
-      }
-      void walkFunction(Function* func) {
-        changed = false;
-        PostWalker<Trapper>::walkFunction(func);
-        if (changed) {
-          ReFinalize().walkFunctionInModule(func, getModule());
-        }
-      }
-      bool isFunctionParallel() override { return true; }
-      Pass* create() override { return new Trapper; }
-    } trapper;
-    trapper.run(runner, module);
-  }
 };
 
 Pass* createMemoryPackingPass() { return new MemoryPacking(); }
diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp
index 8098116be..24f301110 100644
--- a/src/wasm/wasm-validator.cpp
+++ b/src/wasm/wasm-validator.cpp
@@ -970,6 +970,8 @@ void FunctionValidator::visitSIMDShift(SIMDShift* curr) {
 }
 
 void FunctionValidator::visitMemoryInit(MemoryInit* curr) {
+  shouldBeTrue(
+    getModule()->memory.exists, curr, "Memory operations require a memory");
   shouldBeTrue(getModule()->features.hasBulkMemory(),
                curr,
                "Bulk memory operation (bulk memory is disabled)");
@@ -981,33 +983,27 @@ void FunctionValidator::visitMemoryInit(MemoryInit* curr) {
     curr->offset->type, i32, curr, "memory.init offset must be an i32");
   shouldBeEqualOrFirstIsUnreachable(
     curr->size->type, i32, curr, "memory.init size must be an i32");
-  if (!shouldBeTrue(getModule()->memory.exists,
-                    curr,
-                    "Memory operations require a memory")) {
-    return;
-  }
   shouldBeTrue(curr->segment < getModule()->memory.segments.size(),
                curr,
                "memory.init segment index out of bounds");
 }
 
 void FunctionValidator::visitDataDrop(DataDrop* curr) {
+  shouldBeTrue(
+    getModule()->memory.exists, curr, "Memory operations require a memory");
   shouldBeTrue(getModule()->features.hasBulkMemory(),
                curr,
                "Bulk memory operation (bulk memory is disabled)");
   shouldBeEqualOrFirstIsUnreachable(
     curr->type, none, curr, "data.drop must have type none");
-  if (!shouldBeTrue(getModule()->memory.exists,
-                    curr,
-                    "Memory operations require a memory")) {
-    return;
-  }
   shouldBeTrue(curr->segment < getModule()->memory.segments.size(),
                curr,
                "data.drop segment index out of bounds");
 }
 
 void FunctionValidator::visitMemoryCopy(MemoryCopy* curr) {
+  shouldBeTrue(
+    getModule()->memory.exists, curr, "Memory operations require a memory");
   shouldBeTrue(getModule()->features.hasBulkMemory(),
                curr,
                "Bulk memory operation (bulk memory is disabled)");
@@ -1019,11 +1015,11 @@ void FunctionValidator::visitMemoryCopy(MemoryCopy* curr) {
     curr->source->type, i32, curr, "memory.copy source must be an i32");
   shouldBeEqualOrFirstIsUnreachable(
     curr->size->type, i32, curr, "memory.copy size must be an i32");
-  shouldBeTrue(
-    getModule()->memory.exists, curr, "Memory operations require a memory");
 }
 
 void FunctionValidator::visitMemoryFill(MemoryFill* curr) {
+  shouldBeTrue(
+    getModule()->memory.exists, curr, "Memory operations require a memory");
   shouldBeTrue(getModule()->features.hasBulkMemory(),
                curr,
                "Bulk memory operation (bulk memory is disabled)");
@@ -1035,8 +1031,6 @@ void FunctionValidator::visitMemoryFill(MemoryFill* curr) {
     curr->value->type, i32, curr, "memory.fill value must be an i32");
   shouldBeEqualOrFirstIsUnreachable(
     curr->size->type, i32, curr, "memory.fill size must be an i32");
-  shouldBeTrue(
-    getModule()->memory.exists, curr, "Memory operations require a memory");
 }
 
 void FunctionValidator::validateMemBytes(uint8_t bytes,