diff --git a/src/binaryen-c.cpp b/src/binaryen-c.cpp index a8f39c5abdc..cd01d25e0c4 100644 --- a/src/binaryen-c.cpp +++ b/src/binaryen-c.cpp @@ -157,7 +157,7 @@ Literal fromBinaryenLiteral(BinaryenLiteral x) { } } if (heapType.isSignature()) { - return Literal::makeFunc(Name(x.func), heapType); + return Literal::makeFunc(Name(x.func), type); } assert(heapType.isData()); WASM_UNREACHABLE("TODO: gc data"); diff --git a/src/ir/possible-contents.cpp b/src/ir/possible-contents.cpp index aba87d79db2..9fbefa030d1 100644 --- a/src/ir/possible-contents.cpp +++ b/src/ir/possible-contents.cpp @@ -27,6 +27,7 @@ #include "ir/module-utils.h" #include "ir/possible-contents.h" #include "support/insert_ordered.h" +#include "wasm-type.h" #include "wasm.h" namespace std { @@ -641,9 +642,9 @@ struct InfoCollector addRoot(curr); } void visitRefFunc(RefFunc* curr) { - addRoot(curr, - PossibleContents::literal( - Literal::makeFunc(curr->func, curr->type.getHeapType()))); + addRoot( + curr, + PossibleContents::literal(Literal::makeFunc(curr->func, *getModule()))); // The presence of a RefFunc indicates the function may be called // indirectly, so add the relevant connections for this particular function. @@ -1859,8 +1860,8 @@ void TNHOracle::infer() { // as other opts will make this call direct later, after which a // lot of other optimizations become possible anyhow. auto target = possibleTargets[0]->name; - info.inferences[call->target] = PossibleContents::literal( - Literal::makeFunc(target, wasm.getFunction(target)->type)); + info.inferences[call->target] = + PossibleContents::literal(Literal::makeFunc(target, wasm)); continue; } diff --git a/src/ir/properties.h b/src/ir/properties.h index 06209f7091a..8b2938378aa 100644 --- a/src/ir/properties.h +++ b/src/ir/properties.h @@ -116,7 +116,7 @@ inline Literal getLiteral(const Expression* curr) { } else if (auto* n = curr->dynCast()) { return Literal(n->type); } else if (auto* r = curr->dynCast()) { - return Literal::makeFunc(r->func, r->type.getHeapType()); + return Literal::makeFunc(r->func, r->type); } else if (auto* i = curr->dynCast()) { if (auto* c = i->value->dynCast()) { return Literal::makeI31(c->value.geti32(), diff --git a/src/literal.h b/src/literal.h index f20213e0529..92b7ce7689f 100644 --- a/src/literal.h +++ b/src/literal.h @@ -30,6 +30,7 @@ namespace wasm { +class Module; class Literals; struct FuncData; struct GCData; @@ -70,6 +71,9 @@ class Literal { public: // Type of the literal. Immutable because the literal's payload depends on it. + // For references to defined heap types, this is almost always an exact type. + // The exception is references to imported functions, since the function + // provided at instantiation time may have a subtype of the import type. const Type type; Literal() : v128(), type(Type::none) {} @@ -90,7 +94,7 @@ class Literal { explicit Literal(const std::array&); explicit Literal(const std::array&); explicit Literal(const std::array&); - explicit Literal(std::shared_ptr funcData, HeapType type); + explicit Literal(std::shared_ptr funcData, Type type); explicit Literal(std::shared_ptr gcData, HeapType type); explicit Literal(std::shared_ptr exnData); explicit Literal(std::shared_ptr contData); @@ -252,7 +256,8 @@ class Literal { } // Simple way to create a function from the name and type, without a full // FuncData. - static Literal makeFunc(Name func, HeapType type); + static Literal makeFunc(Name func, Type type); + static Literal makeFunc(Name func, Module& wasm); static Literal makeI31(int32_t value, Shareability share) { auto lit = Literal(Type(HeapTypes::i31.getBasic(share), NonNullable)); lit.i32 = value | 0x80000000; diff --git a/src/shell-interface.h b/src/shell-interface.h index 11abe6bac55..b059ecf2b3b 100644 --- a/src/shell-interface.h +++ b/src/shell-interface.h @@ -148,7 +148,7 @@ struct ShellExternalInterface : ModuleRunner::ExternalInterface { } return Flow(); }), - import->type); + Type(import->type, NonNullable, Exact)); } else if (import->module == ENV && import->base == EXIT) { return Literal(std::make_shared(import->name, nullptr, @@ -157,7 +157,7 @@ struct ShellExternalInterface : ModuleRunner::ExternalInterface { std::cout << "exit()\n"; throw ExitException(); }), - import->type); + Type(import->type, NonNullable, Exact)); } else if (auto* inst = getImportInstance(import)) { return inst->getExportedFunction(import->base); } diff --git a/src/tools/execution-results.h b/src/tools/execution-results.h index aad6ed35102..d06d0e03aa0 100644 --- a/src/tools/execution-results.h +++ b/src/tools/execution-results.h @@ -202,7 +202,7 @@ struct LoggingExternalInterface : public ShellExternalInterface { }; // Use a null instance because this is a host function. return Literal(std::make_shared(import->name, nullptr, f), - import->type); + Type(import->type, NonNullable, Exact)); } void throwJSException() { diff --git a/src/tools/wasm-ctor-eval.cpp b/src/tools/wasm-ctor-eval.cpp index 8abdcc9e666..ca904926230 100644 --- a/src/tools/wasm-ctor-eval.cpp +++ b/src/tools/wasm-ctor-eval.cpp @@ -103,7 +103,7 @@ class EvallingModuleRunner : public ModuleRunnerBase { } // This needs to be duplicated from ModuleRunner, unfortunately. - Literal makeFuncData(Name name, HeapType type) { + Literal makeFuncData(Name name, Type type) { auto allocation = std::make_shared(name, this, [this, name](Literals arguments) { return callFunction(name, arguments); @@ -319,8 +319,9 @@ struct CtorEvalExternalInterface : EvallingModuleRunner::ExternalInterface { }; // Use a null instance because these are either host functions or imported // from unknown sources. + // TODO: Be more precise about the types we allow these imports to have. return Literal(std::make_shared(import->name, nullptr, f), - import->type); + Type(import->type, NonNullable, Exact)); } // We assume the table is not modified FIXME diff --git a/src/wasm-interpreter.h b/src/wasm-interpreter.h index 4fe7cea6bbf..2a751378bf2 100644 --- a/src/wasm-interpreter.h +++ b/src/wasm-interpreter.h @@ -346,7 +346,7 @@ class ExpressionRunner : public OverriddenVisitor { Execute, }; - Literal makeFuncData(Name name, HeapType type) { + Literal makeFuncData(Name name, Type type) { // Identify the interpreter, but do not provide a way to actually call the // function. auto allocation = std::make_shared(name, this); @@ -1870,7 +1870,7 @@ class ExpressionRunner : public OverriddenVisitor { return Literal(int32_t(value.isNull())); } Flow visitRefFunc(RefFunc* curr) { - return self()->makeFuncData(curr->func, curr->type.getHeapType()); + return self()->makeFuncData(curr->func, curr->type); } Flow visitRefEq(RefEq* curr) { VISIT(flow, curr->left) @@ -3177,7 +3177,7 @@ class ModuleRunnerBase : public ExpressionRunner { [this, func](const Literals& arguments) -> Flow { return callFunction(func->name, arguments); }), - func->type); + Type(func->type, NonNullable, Exact)); } // get an exported global @@ -3459,12 +3459,12 @@ class ModuleRunnerBase : public ExpressionRunner { Literals arguments; VISIT_ARGUMENTS(flow, curr->operands, arguments); auto* func = wasm.getFunction(curr->target); - auto funcType = func->type; + auto funcType = Type(func->type, NonNullable, Exact); if (Intrinsics(*self()->getModule()).isCallWithoutEffects(func)) { // The call.without.effects intrinsic is a call to an import that actually // calls the given function reference that is the final argument. target = arguments.back().getFunc(); - funcType = arguments.back().type.getHeapType(); + funcType = funcType.with(arguments.back().type.getHeapType()); arguments.pop_back(); } @@ -4419,8 +4419,9 @@ class ModuleRunnerBase : public ExpressionRunner { } auto funcName = funcValue.getFunc(); auto* func = self()->getModule()->getFunction(funcName); + auto funcType = Type(func->type, NonNullable, Exact); return Literal(std::make_shared( - self()->makeFuncData(func->name, func->type), curr->type.getHeapType())); + self()->makeFuncData(funcName, funcType), curr->type.getHeapType())); } Flow visitContBind(ContBind* curr) { Literals arguments; @@ -4767,7 +4768,8 @@ class ModuleRunnerBase : public ExpressionRunner { // not the original function that was called, and the original has been // returned from already; we should call the last return_called // function). - auto target = self()->makeFuncData(name, function->type); + auto funcType = Type(function->type, NonNullable, Exact); + auto target = self()->makeFuncData(name, funcType); self()->pushResumeEntry({target}, "function-target"); } @@ -4916,7 +4918,7 @@ class ModuleRunner : public ModuleRunnerBase { std::map> linkedInstances = {}) : ModuleRunnerBase(wasm, externalInterface, linkedInstances) {} - Literal makeFuncData(Name name, HeapType type) { + Literal makeFuncData(Name name, Type type) { // As the super's |makeFuncData|, but here we also provide a way to // actually call the function. auto allocation = diff --git a/src/wasm/literal.cpp b/src/wasm/literal.cpp index 98d7c61d383..bfec71c6447 100644 --- a/src/wasm/literal.cpp +++ b/src/wasm/literal.cpp @@ -71,17 +71,23 @@ Literal::Literal(const uint8_t init[16]) : type(Type::v128) { memcpy(&v128, init, 16); } -Literal::Literal(std::shared_ptr funcData, HeapType type) - : funcData(funcData), type(type, NonNullable, Exact) { +Literal::Literal(std::shared_ptr funcData, Type type) + : funcData(funcData), type(type) { assert(funcData); assert(type.isSignature()); } -Literal Literal::makeFunc(Name func, HeapType type) { +Literal Literal::makeFunc(Name func, Type type) { // Provide only the name of the function, without execution info. return Literal(std::make_shared(func), type); } +Literal Literal::makeFunc(Name func, Module& wasm) { + auto* f = wasm.getFunction(func); + auto exact = f->imported() ? Inexact : Exact; + return makeFunc(func, Type(f->type, NonNullable, exact)); +} + Literal::Literal(std::shared_ptr gcData, HeapType type) : gcData(gcData), type(type, gcData ? NonNullable : Nullable, diff --git a/test/gtest/possible-contents.cpp b/test/gtest/possible-contents.cpp index 2633bdb3714..e1c56b99879 100644 --- a/test/gtest/possible-contents.cpp +++ b/test/gtest/possible-contents.cpp @@ -89,8 +89,8 @@ class PossibleContentsTest : public testing::Test { PossibleContents nonNullFuncGlobal = PossibleContents::global("funcGlobal", Type(HeapType::func, NonNullable)); - PossibleContents nonNullFunc = PossibleContents::literal( - Literal::makeFunc("func", Signature(Type::none, Type::none))); + PossibleContents nonNullFunc = PossibleContents::literal(Literal::makeFunc( + "func", Type(Signature(Type::none, Type::none), NonNullable, Exact))); PossibleContents exactI32 = PossibleContents::exactType(Type::i32); PossibleContents exactAnyref = PossibleContents::exactType(anyref); diff --git a/test/lit/passes/gufa.wast b/test/lit/passes/gufa.wast index 721c27eeb66..7d20881b33e 100644 --- a/test/lit/passes/gufa.wast +++ b/test/lit/passes/gufa.wast @@ -1154,3 +1154,108 @@ ) ) ) + +;; We cannot know the types of imported functions, so we should not be able to +;; optimize this exact cast. +(module + ;; CHECK: (type $func (sub (func))) + (type $func (sub (func))) + (type $sub (sub $func (func))) + ;; CHECK: (type $1 (func (result i32))) + + ;; CHECK: (import "" "" (func $f (type $func))) + (import "" "" (func $f (type $func))) + ;; CHECK: (elem declare func $f) + + ;; CHECK: (export "test" (func $test)) + + ;; CHECK: (func $test (type $1) (result i32) + ;; CHECK-NEXT: (ref.test (ref (exact $func)) + ;; CHECK-NEXT: (ref.func $f) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test (export "test") (result i32) + (ref.test (ref (exact $func)) + (ref.func $f) + ) + ) +) + +;; As above, but now the cast is to a subtype. We should not be able to optimize +;; this either. +;; TODO: We misoptimize this. Fix it! +(module + ;; CHECK: (type $func (sub (func))) + (type $func (sub (func))) + ;; CHECK: (type $1 (func (result i32))) + + ;; CHECK: (type $sub (sub $func (func))) + (type $sub (sub $func (func))) + ;; CHECK: (import "" "" (func $f (type $func))) + (import "" "" (func $f (type $func))) + ;; CHECK: (export "test" (func $test)) + + ;; CHECK: (func $test (type $1) (result i32) + ;; CHECK-NEXT: (ref.test (ref $sub) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test (export "test") (result i32) + (ref.test (ref $sub) + ;; TODO: Remove this block. It is currently necessary because we + ;; incorrectly type the function reference as exact, so without the block + ;; the cast type above would be "improved" to (ref nofunc). + (block (result (ref $func)) + (ref.func $f) + ) + ) + ) +) + +;; This is another exact cast, but now the imported type is final, so we know +;; it will succeed. +(module + ;; CHECK: (type $func (func)) + (type $func (sub final (func))) + ;; CHECK: (type $1 (func (result i32))) + + ;; CHECK: (import "" "" (func $f (type $func))) + (import "" "" (func $f (type $func))) + ;; CHECK: (elem declare func $f) + + ;; CHECK: (export "test" (func $test)) + + ;; CHECK: (func $test (type $1) (result i32) + ;; CHECK-NEXT: (ref.test (ref (exact $func)) + ;; CHECK-NEXT: (ref.func $f) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test (export "test") (result i32) + (ref.test (ref (exact $func)) + (ref.func $f) + ) + ) +) + +;; Now we use a ref.cast instead of a ref.test with the exact cast to the final +;; type. We cannot optimize even though we know the cast will succeed because +;; the Wasm type of the function reference is inexact. +;; TODO: We misoptimize here, too! +(module + ;; CHECK: (type $func (func)) + (type $func (sub final (func))) + ;; CHECK: (type $1 (func (result funcref))) + + ;; CHECK: (import "" "" (func $f (type $func))) + (import "" "" (func $f (type $func))) + ;; CHECK: (export "test" (func $test)) + + ;; CHECK: (func $test (type $1) (result funcref) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $test (export "test") (result funcref) + (ref.cast (ref (exact $func)) + (ref.func $f) + ) + ) +)