[ctor-eval] Stop if there are any memory.init instructions (#4442)

This tool depends (atm) on flattening memory segments. That is not compatible
with memory.init which cares about segment identities.

This changes flatten() only by adding the check for MemoryInit. The rest is
unchanged, although I saw the other two params are not needed and I removed
them while I was there.

Author: kripken

src/ir/CMakeLists.txt

@@ -5,6 +5,7 @@ set(ir_SOURCES
   eh-utils.cpp
   intrinsics.cpp
   lubs.cpp
+  memory-utils.cpp
   names.cpp
   properties.cpp
   LocalGraph.cpp

src/ir/memory-utils.cpp

@@ -0,0 +1,72 @@
+/*
+ * Copyright 2022 WebAssembly Community Group participants
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "ir/memory-utils.h"
+#include "wasm.h"
+
+namespace wasm::MemoryUtils {
+
+bool flatten(Module& wasm) {
+  // The presence of any MemoryInit instructions is a problem because they care
+  // about segment identity, which flattening gets rid of ( when it merges them
+  // all into one big segment).
+  ModuleUtils::ParallelFunctionAnalysis<bool> analysis(
+    wasm, [&](Function* func, bool& hasMemoryInit) {
+      if (func->imported()) {
+        return;
+      }
+      hasMemoryInit = FindAll<MemoryInit>(func->body).list.size() > 0;
+    });
+
+  for (auto& [func, hasMemoryInit] : analysis.map) {
+    if (hasMemoryInit) {
+      return false;
+    }
+  }
+
+  auto& memory = wasm.memory;
+
+  if (memory.segments.size() == 0) {
+    return true;
+  }
+
+  std::vector<char> data;
+  for (auto& segment : memory.segments) {
+    if (segment.isPassive) {
+      return false;
+    }
+    auto* offset = segment.offset->dynCast<Const>();
+    if (!offset) {
+      return false;
+    }
+  }
+  for (auto& segment : memory.segments) {
+    auto* offset = segment.offset->dynCast<Const>();
+    Index start = offset->value.getInteger();
+    Index end = start + segment.data.size();
+    if (end > data.size()) {
+      data.resize(end);
+    }
+    std::copy(segment.data.begin(), segment.data.end(), data.begin() + start);
+  }
+  memory.segments.resize(1);
+  memory.segments[0].offset->cast<Const>()->value = Literal(int32_t(0));
+  memory.segments[0].data.swap(data);
+
+  return true;
+}
+
+} // namespace wasm::MemoryUtils

src/ir/memory-utils.h

@@ -29,46 +29,8 @@ namespace wasm::MemoryUtils {
 
 // Flattens memory into a single data segment, or no segment. If there is
 // a segment, it starts at 0.
-// If ensuredSegmentSize is provided, then a segment is always emitted,
-// and of at least that size.
 // Returns true if successful (e.g. relocatable segments cannot be flattened).
-inline bool flatten(Memory& memory,
-                    Index ensuredSegmentSize = 0,
-                    Module* module = nullptr) {
-  if (memory.segments.size() == 0) {
-    if (ensuredSegmentSize > 0) {
-      assert(module); // must provide a module if ensuring a size.
-      Builder builder(*module);
-      memory.segments.emplace_back(builder.makeConst(int32_t(0)));
-      memory.segments[0].data.resize(ensuredSegmentSize);
-    }
-    return true;
-  }
-  std::vector<char> data;
-  data.resize(ensuredSegmentSize);
-  for (auto& segment : memory.segments) {
-    if (segment.isPassive) {
-      return false;
-    }
-    auto* offset = segment.offset->dynCast<Const>();
-    if (!offset) {
-      return false;
-    }
-  }
-  for (auto& segment : memory.segments) {
-    auto* offset = segment.offset->dynCast<Const>();
-    Index start = offset->value.getInteger();
-    Index end = start + segment.data.size();
-    if (end > data.size()) {
-      data.resize(end);
-    }
-    std::copy(segment.data.begin(), segment.data.end(), data.begin() + start);
-  }
-  memory.segments.resize(1);
-  memory.segments[0].offset->cast<Const>()->value = Literal(int32_t(0));
-  memory.segments[0].data.swap(data);
-  return true;
-}
+bool flatten(Module& wasm);
 
 // Ensures that the memory exists (of minimal size).
 inline void ensureExists(Memory& memory) {

src/tools/wasm-ctor-eval.cpp

@@ -663,11 +663,6 @@ void evalCtors(Module& wasm,
 
   CtorEvalExternalInterface interface(linkedInstances);
   try {
-    // flatten memory, so we do not depend on the layout of data segments
-    if (!MemoryUtils::flatten(wasm.memory)) {
-      Fatal() << "  ...stopping since could not flatten memory\n";
-    }
-
     // create an instance for evalling
     EvallingModuleInstance instance(wasm, &interface, linkedInstances);
     // we should not add new globals from here on; as a result, using
@@ -712,6 +707,16 @@ void evalCtors(Module& wasm,
   }
 }
 
+static bool canEval(Module& wasm) {
+  // Check if we can flatten memory. We need to do so currently because of how
+  // we assume memory is simple and flat. TODO
+  if (!MemoryUtils::flatten(wasm)) {
+    std::cout << "  ...stopping since could not flatten memory\n";
+    return false;
+  }
+  return true;
+}
+
 } // anonymous namespace
 
 //
@@ -808,19 +813,21 @@ int main(int argc, const char* argv[]) {
     Fatal() << "error in validating input";
   }
 
-  evalCtors(wasm, ctors, keptExports);
-
-  // Do some useful optimizations after the evalling
-  {
-    PassRunner passRunner(&wasm);
-    passRunner.add("memory-packing"); // we flattened it, so re-optimize
-    // TODO: just do -Os for the one function
-    passRunner.add("remove-unused-names");
-    passRunner.add("dce");
-    passRunner.add("merge-blocks");
-    passRunner.add("vacuum");
-    passRunner.add("remove-unused-module-elements");
-    passRunner.run();
+  if (canEval(wasm)) {
+    evalCtors(wasm, ctors, keptExports);
+
+    // Do some useful optimizations after the evalling
+    {
+      PassRunner passRunner(&wasm);
+      passRunner.add("memory-packing"); // we flattened it, so re-optimize
+      // TODO: just do -Os for the one function
+      passRunner.add("remove-unused-names");
+      passRunner.add("dce");
+      passRunner.add("merge-blocks");
+      passRunner.add("vacuum");
+      passRunner.add("remove-unused-module-elements");
+      passRunner.run();
+    }
   }
 
   if (options.extra.count("output") > 0) {

test/ctor-eval/memory-init.wast

@@ -0,0 +1,22 @@
+(module
+  (memory $0 1)
+  (data (i32.const 0) "__________")
+  (data (i32.const 20) "__________")
+  (func "test1"
+    ;; A store that can be evalled.
+    (i32.store8
+      (i32.const 4)
+      (i32.const 100)
+    )
+
+    ;; A memory init cannot be evalled since ctor-eval flattens memory segments
+    ;; atm. We lose the identity of them as a result as they are all merged
+    ;; into a single big segment. We could fix that eventually.
+    (memory.init 1
+      (i32.const 0)
+      (i32.const 0)
+      (i32.const 1)
+    )
+  )
+)
+

test/ctor-eval/memory-init.wast.ctors

@@ -0,0 +1 @@
+test1

test/ctor-eval/memory-init.wast.out

@@ -0,0 +1,18 @@
+(module
+ (type $none_=>_none (func))
+ (memory $0 1)
+ (data (i32.const 0) "__________")
+ (data (i32.const 20) "__________")
+ (export "test1" (func $0))
+ (func $0
+  (i32.store8
+   (i32.const 4)
+   (i32.const 100)
+  )
+  (memory.init 1
+   (i32.const 0)
+   (i32.const 0)
+   (i32.const 1)
+  )
+ )
+)