add docs and error hints when a Call should be a CallImport (#1081)

* add docs and error hints when a Call should be a CallImport

* fix binaryen API docs in docs/

Author: kripken

bin/binaryen.js

@@ -56,15 +56,19 @@ else
   # See https://gist.github.com/rsms/e33c61a25a31c08260161a087be03169
 fi
 
-echo "building wasm.js"
+if [ "$1" != "-g" ]; then
+  EMCC_ARGS="$EMCC_ARGS --closure 1"
+fi
+
+echo "building shared bitcode"
 
 "$EMSCRIPTEN/em++" \
   $EMCC_ARGS \
-  src/wasm-js.cpp \
   src/ast/ExpressionAnalyzer.cpp \
   src/ast/ExpressionManipulator.cpp \
   src/passes/pass.cpp \
   src/passes/CoalesceLocals.cpp \
+  src/passes/CodeFolding.cpp \
   src/passes/CodePushing.cpp \
   src/passes/DeadCodeElimination.cpp \
   src/passes/DuplicateFunctionElimination.cpp \
@@ -80,7 +84,6 @@ echo "building wasm.js"
   src/passes/MergeBlocks.cpp \
   src/passes/Metrics.cpp \
   src/passes/NameList.cpp \
-  src/passes/NameManager.cpp \
   src/passes/OptimizeInstructions.cpp \
   src/passes/PickLoadSigns.cpp \
   src/passes/PostEmscripten.cpp \
@@ -117,84 +120,27 @@ echo "building wasm.js"
   src/wasm/literal.cpp \
   src/cfg/Relooper.cpp \
   -Isrc/ \
-  -o bin/wasm${OUT_FILE_SUFFIX}.js \
-  -s MODULARIZE=1 \
-  -s 'EXPORT_NAME="WasmJS"'
-  #-DWASM_JS_DEBUG
-  #-DWASM_INTERPRETER_DEBUG=2
-
-echo "building binaryen.js to bitcode"
+  -o shared.bc
 
-if [ "$1" != "-g" ]; then
-  EMCC_ARGS="$EMCC_ARGS --closure 1"
-fi
+echo "building wasm.js"
 
 "$EMSCRIPTEN/em++" \
   $EMCC_ARGS \
-  -std=c++11 \
-  src/ast/ExpressionAnalyzer.cpp \
-  src/ast/ExpressionManipulator.cpp \
-  src/passes/pass.cpp \
-  src/passes/CoalesceLocals.cpp \
-  src/passes/CodePushing.cpp \
-  src/passes/DeadCodeElimination.cpp \
-  src/passes/DuplicateFunctionElimination.cpp \
-  src/passes/ExtractFunction.cpp \
-  src/passes/FlattenControlFlow.cpp \
-  src/passes/Inlining.cpp \
-  src/passes/InstrumentLocals.cpp \
-  src/passes/InstrumentMemory.cpp \
-  src/passes/LegalizeJSInterface.cpp \
-  src/passes/LocalCSE.cpp \
-  src/passes/LogExecution.cpp \
-  src/passes/MemoryPacking.cpp \
-  src/passes/MergeBlocks.cpp \
-  src/passes/Metrics.cpp \
-  src/passes/NameList.cpp \
-  src/passes/NameManager.cpp \
-  src/passes/OptimizeInstructions.cpp \
-  src/passes/PickLoadSigns.cpp \
-  src/passes/PostEmscripten.cpp \
-  src/passes/Precompute.cpp \
-  src/passes/PrintCallGraph.cpp \
-  src/passes/Print.cpp \
-  src/passes/RelooperJumpThreading.cpp \
-  src/passes/RemoveImports.cpp \
-  src/passes/RemoveMemory.cpp \
-  src/passes/RemoveUnusedBrs.cpp \
-  src/passes/RemoveUnusedModuleElements.cpp \
-  src/passes/RemoveUnusedNames.cpp \
-  src/passes/ReorderFunctions.cpp \
-  src/passes/ReorderLocals.cpp \
-  src/passes/ReReloop.cpp \
-  src/passes/SSAify.cpp \
-  src/passes/SimplifyLocals.cpp \
-  src/passes/Untee.cpp \
-  src/passes/Vacuum.cpp \
-  src/wasm-emscripten.cpp \
-  src/support/colors.cpp \
-  src/support/safe_integer.cpp \
-  src/support/bits.cpp \
-  src/support/threads.cpp \
-  src/asmjs/asm_v_wasm.cpp \
-  src/asmjs/shared-constants.cpp \
-  src/wasm/wasm.cpp \
-  src/wasm/wasm-type.cpp \
-  src/wasm/wasm-binary.cpp \
-  src/wasm/wasm-s-parser.cpp \
-  src/wasm/literal.cpp \
-  src/binaryen-c.cpp \
-  src/cfg/Relooper.cpp \
-  -o binaryen.bc \
-  -Isrc/
+  src/wasm-js.cpp \
+  shared.bc \
+  -Isrc/ \
+  -o bin/wasm${OUT_FILE_SUFFIX}.js \
+  -s MODULARIZE=1 \
+  -s 'EXPORT_NAME="WasmJS"'
 
-echo "building binaryen.js to js"
+echo "building binaryen.js"
 
 "$EMSCRIPTEN/em++" \
   $EMCC_ARGS \
-  binaryen.bc \
+  src/binaryen-c.cpp \
+  shared.bc \
+  -Isrc/ \
   -s 'EXPORTED_FUNCTIONS=["_BinaryenNone", "_BinaryenInt32", "_BinaryenInt64", "_BinaryenFloat32", "_BinaryenFloat64", "_BinaryenUndefined", "_BinaryenModuleCreate", "_BinaryenModuleDispose", "_BinaryenAddFunctionType", "_BinaryenGetFunctionTypeBySignature", "_BinaryenLiteralInt32", "_BinaryenLiteralInt64", "_BinaryenLiteralFloat32", "_BinaryenLiteralFloat64", "_BinaryenLiteralFloat32Bits", "_BinaryenLiteralFloat64Bits", "_BinaryenClzInt32", "_BinaryenCtzInt32", "_BinaryenPopcntInt32", "_BinaryenNegFloat32", "_BinaryenAbsFloat32", "_BinaryenCeilFloat32", "_BinaryenFloorFloat32", "_BinaryenTruncFloat32", "_BinaryenNearestFloat32", "_BinaryenSqrtFloat32", "_BinaryenEqZInt32", "_BinaryenClzInt64", "_BinaryenCtzInt64", "_BinaryenPopcntInt64", "_BinaryenNegFloat64", "_BinaryenAbsFloat64", "_BinaryenCeilFloat64", "_BinaryenFloorFloat64", "_BinaryenTruncFloat64", "_BinaryenNearestFloat64", "_BinaryenSqrtFloat64", "_BinaryenEqZInt64", "_BinaryenExtendSInt32", "_BinaryenExtendUInt32", "_BinaryenWrapInt64", "_BinaryenTruncSFloat32ToInt32", "_BinaryenTruncSFloat32ToInt64", "_BinaryenTruncUFloat32ToInt32", "_BinaryenTruncUFloat32ToInt64", "_BinaryenTruncSFloat64ToInt32", "_BinaryenTruncSFloat64ToInt64", "_BinaryenTruncUFloat64ToInt32", "_BinaryenTruncUFloat64ToInt64", "_BinaryenReinterpretFloat32", "_BinaryenReinterpretFloat64", "_BinaryenConvertSInt32ToFloat32", "_BinaryenConvertSInt32ToFloat64", "_BinaryenConvertUInt32ToFloat32", "_BinaryenConvertUInt32ToFloat64", "_BinaryenConvertSInt64ToFloat32", "_BinaryenConvertSInt64ToFloat64", "_BinaryenConvertUInt64ToFloat32", "_BinaryenConvertUInt64ToFloat64", "_BinaryenPromoteFloat32", "_BinaryenDemoteFloat64", "_BinaryenReinterpretInt32", "_BinaryenReinterpretInt64", "_BinaryenAddInt32", "_BinaryenSubInt32", "_BinaryenMulInt32", "_BinaryenDivSInt32", "_BinaryenDivUInt32", "_BinaryenRemSInt32", "_BinaryenRemUInt32", "_BinaryenAndInt32", "_BinaryenOrInt32", "_BinaryenXorInt32", "_BinaryenShlInt32", "_BinaryenShrUInt32", "_BinaryenShrSInt32", "_BinaryenRotLInt32", "_BinaryenRotRInt32", "_BinaryenEqInt32", "_BinaryenNeInt32", "_BinaryenLtSInt32", "_BinaryenLtUInt32", "_BinaryenLeSInt32", "_BinaryenLeUInt32", "_BinaryenGtSInt32", "_BinaryenGtUInt32", "_BinaryenGeSInt32", "_BinaryenGeUInt32", "_BinaryenAddInt64", "_BinaryenSubInt64", "_BinaryenMulInt64", "_BinaryenDivSInt64", "_BinaryenDivUInt64", "_BinaryenRemSInt64", "_BinaryenRemUInt64", "_BinaryenAndInt64", "_BinaryenOrInt64", "_BinaryenXorInt64", "_BinaryenShlInt64", "_BinaryenShrUInt64", "_BinaryenShrSInt64", "_BinaryenRotLInt64", "_BinaryenRotRInt64", "_BinaryenEqInt64", "_BinaryenNeInt64", "_BinaryenLtSInt64", "_BinaryenLtUInt64", "_BinaryenLeSInt64", "_BinaryenLeUInt64", "_BinaryenGtSInt64", "_BinaryenGtUInt64", "_BinaryenGeSInt64", "_BinaryenGeUInt64", "_BinaryenAddFloat32", "_BinaryenSubFloat32", "_BinaryenMulFloat32", "_BinaryenDivFloat32", "_BinaryenCopySignFloat32", "_BinaryenMinFloat32", "_BinaryenMaxFloat32", "_BinaryenEqFloat32", "_BinaryenNeFloat32", "_BinaryenLtFloat32", "_BinaryenLeFloat32", "_BinaryenGtFloat32", "_BinaryenGeFloat32", "_BinaryenAddFloat64", "_BinaryenSubFloat64", "_BinaryenMulFloat64", "_BinaryenDivFloat64", "_BinaryenCopySignFloat64", "_BinaryenMinFloat64", "_BinaryenMaxFloat64", "_BinaryenEqFloat64", "_BinaryenNeFloat64", "_BinaryenLtFloat64", "_BinaryenLeFloat64", "_BinaryenGtFloat64", "_BinaryenGeFloat64", "_BinaryenPageSize", "_BinaryenCurrentMemory", "_BinaryenGrowMemory", "_BinaryenHasFeature", "_BinaryenBlock", "_BinaryenIf", "_BinaryenLoop", "_BinaryenBreak", "_BinaryenSwitch", "_BinaryenCall", "_BinaryenCallImport", "_BinaryenCallIndirect", "_BinaryenGetLocal", "_BinaryenSetLocal", "_BinaryenTeeLocal", "_BinaryenGetGlobal", "_BinaryenSetGlobal", "_BinaryenLoad", "_BinaryenStore", "_BinaryenConst", "_BinaryenUnary", "_BinaryenBinary", "_BinaryenSelect", "_BinaryenDrop", "_BinaryenReturn", "_BinaryenHost", "_BinaryenNop", "_BinaryenUnreachable", "_BinaryenExpressionPrint", "_BinaryenAddFunction", "_BinaryenAddGlobal", "_BinaryenAddImport", "_BinaryenRemoveImport", "_BinaryenAddExport", "_BinaryenRemoveExport", "_BinaryenSetFunctionTable", "_BinaryenSetMemory", "_BinaryenSetStart", "_BinaryenModuleParse", "_BinaryenModulePrint", "_BinaryenModuleValidate", "_BinaryenModuleOptimize", "_BinaryenModuleAutoDrop", "_BinaryenModuleWrite", "_BinaryenModuleRead", "_BinaryenModuleInterpret", "_RelooperCreate", "_RelooperAddBlock", "_RelooperAddBranch", "_RelooperAddBlockWithSwitch", "_RelooperAddBranchForSwitch", "_RelooperRenderAndDispose", "_BinaryenSetAPITracing"]' \
   -o bin/binaryen${OUT_FILE_SUFFIX}.js \
-  --memory-init-file 0 \
   --pre-js src/js/binaryen.js-pre.js \
   --post-js src/js/binaryen.js-post.js

bin/wasm.js

@@ -211,7 +211,7 @@ Unprefixed expressions:
   * `break(label, condition, value)`: Create a break, to a label, and with an optional condition, and optional value.
   * `switch(labels, defaultLabel, condition, value)`: Create a switch (aka br_table), with a list of labels, a default label, a condition, and an optional value.
   * `call(name, operands, type)`: Create a call, to a function name, with operands, and having a specific return type (note that we must specify the return type here as we may not have created the function being called yet, and we may want to optimize this function before we do so, so the API requires that each function be independent of the others, which means that we can't depend on the definition of another function).
-  * `call(name, operands, type)`: Similar to `call`, but calls an imported function.
+  * `callImport(name, operands, type)`: Similar to `call`, but calls an imported function.
   * `callIndirect(target, operands, type)`: Similar to `call`, but calls indirectly, i.e., via a function pointer, so an expression replaces the name as the called value.
   * `getLocal(index, type)`: Create a get_local, for the local at the specified index, and having a specific type (the type is required for the same reasons as in `call`).
   * `setLocal(index, value)`: Create a set_local, for the local at the specified index, and setting the specified value.

build-js.sh

@@ -281,6 +281,9 @@ BinaryenExpressionRef BinaryenSwitch(BinaryenModuleRef module, const char **name
 //                   type returned by the function being called, as that
 //                   function might not have been created yet, so we don't
 //                   know what it is.
+//                   Also note that WebAssembly does not differentiate
+//                   between Call and CallImport, but Binaryen does, so you
+//                   must use CallImport if calling an import, and vice versa.
 BinaryenExpressionRef BinaryenCall(BinaryenModuleRef module, const char *target, BinaryenExpressionRef* operands, BinaryenIndex numOperands, BinaryenType returnType);
 BinaryenExpressionRef BinaryenCallImport(BinaryenModuleRef module, const char *target, BinaryenExpressionRef* operands, BinaryenIndex numOperands, BinaryenType returnType);
 BinaryenExpressionRef BinaryenCallIndirect(BinaryenModuleRef module, BinaryenExpressionRef target, BinaryenExpressionRef* operands, BinaryenIndex numOperands, const char* type);

docs/binaryen.js.Markdown

@@ -168,7 +168,12 @@ void WasmValidator::visitSwitch(Switch *curr) {
 void WasmValidator::visitCall(Call *curr) {
   if (!validateGlobally) return;
   auto* target = getModule()->getFunctionOrNull(curr->target);
-  if (!shouldBeTrue(!!target, curr, "call target must exist")) return;
+  if (!shouldBeTrue(!!target, curr, "call target must exist")) {
+    if (getModule()->getImportOrNull(curr->target)) {
+      std::cerr << "(perhaps it should be a CallImport instead of Call?)\n";
+    }
+    return;
+  }
   if (!shouldBeTrue(curr->operands.size() == target->params.size(), curr, "call param number must match")) return;
   for (size_t i = 0; i < curr->operands.size(); i++) {
     if (!shouldBeEqualOrFirstIsUnreachable(curr->operands[i]->type, target->params[i], curr, "call param types must match")) {

src/binaryen-c.h

@@ -0,0 +1,13 @@
+var module = new Binaryen.Module();
+
+var signature = module.addFunctionType("v", Binaryen.none, []);
+module.addImport("fn", "env", "fn", signature);
+
+module.addFunction("main", signature, [], module.block("", [
+  module.call("fn", [], Binaryen.none) // should be callImport
+]));
+module.addExport("main", "main");
+
+console.log(module.emitText());
+
+module.validate(); // fails

src/wasm/wasm-validator.cpp

@@ -0,0 +1,22 @@
+(module
+ (type $v (func))
+ (import "env" "fn" (func $fn))
+ (memory $0 0)
+ (export "main" (func $main))
+ (func $main (type $v)
+  (call $fn)
+ )
+)
+
+[wasm-validator error in function $main] unexpected false: call target must exist, on 
+(call $fn)
+(perhaps it should be a CallImport instead of Call?)
+(module
+ (type $v (func))
+ (import "env" "fn" (func $fn))
+ (memory $0 0)
+ (export "main" (func $main))
+ (func $main (type $v)
+  (call $fn)
+ )
+)