From 7b64d76b1d339420d79d879032f3e6843c4df3d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Fri, 10 Mar 2023 09:42:29 +0000 Subject: [PATCH] [dart2wasm] Fix const and non-const instantiation closure comparison MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes closure equality when one of the closures is const instantiation and the other one is runtime instantiation. New passing test: language/closure/identity_equality_tearoff_test Change-Id: I4583f9823eb484425208c217a2df585c6fb56d70 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/287462 Reviewed-by: Aske Simon Christensen Commit-Queue: Ömer Ağacan --- pkg/dart2wasm/lib/constants.dart | 15 ++++- pkg/dart2wasm/lib/intrinsics.dart | 105 +++++++++++++++++++----------- 2 files changed, 80 insertions(+), 40 deletions(-) diff --git a/pkg/dart2wasm/lib/constants.dart b/pkg/dart2wasm/lib/constants.dart index f2c594249f6..bf7ad2624ce 100644 --- a/pkg/dart2wasm/lib/constants.dart +++ b/pkg/dart2wasm/lib/constants.dart @@ -582,9 +582,14 @@ class ConstantCreator extends ConstantVisitor { tearOffConstant.function.namedParameters.map((p) => p.name!).toList(); ClosureRepresentation representation = translator.closureLayouter .getClosureRepresentation(0, positionalCount, names)!; + ClosureRepresentation instantiationRepresentation = translator + .closureLayouter + .getClosureRepresentation(types.length, positionalCount, names)!; w.StructType struct = representation.closureStruct; w.RefType type = w.RefType.def(struct, nullable: false); + final tearOffConstantInfo = ensureConstant(tearOffConstant)!; + w.DefinedFunction makeDynamicCallEntry() { final w.DefinedFunction function = m.addFunction( translator.dynamicCallVtableEntryFunctionType, "dynamic call entry"); @@ -671,7 +676,15 @@ class ConstantCreator extends ConstantVisitor { b.i32_const(info.classId); b.i32_const(initialIdentityHash); - b.global_get(translator.globals.dummyStructGlobal); // Dummy context + + // Context is not used by the vtable functions, but it's needed for + // closure equality checks to work (`_Closure._equals`). + b.global_get(tearOffConstantInfo.global); + for (final ty in types) { + b.global_get(ty.global); + } + b.struct_new(instantiationRepresentation.instantiationContextStruct!); + makeVtable(); constants.instantiateConstant( function, b, functionTypeConstant, this.types.nonNullableTypeType); diff --git a/pkg/dart2wasm/lib/intrinsics.dart b/pkg/dart2wasm/lib/intrinsics.dart index 044d29cb2de..e256e4dcf73 100644 --- a/pkg/dart2wasm/lib/intrinsics.dart +++ b/pkg/dart2wasm/lib/intrinsics.dart @@ -1596,23 +1596,28 @@ class Intrinsifier { // // bool _equals(f1, f2) { // if (identical(f1, f2) return true; - // if (f1.vtable == f2.vtable) { - // if (f1 and f2 are instantiations) { - // if (f1.context.inner.vtable == f2.context.inner.vtable) { - // if (typesEqual(f1.context, f2.context)) { - // f1 = f1.context.inner; - // f2 = f2.context.inner; - // if (identical(f1, f2)) return true; - // goto outerClosureContext; - // } - // } - // return false; - // } - // outerClosureContext: - // if (f1.context is #Top && f2.context is #Top) { - // return identical(f1.context, f2.context); - // } + // + // if ( + // ? f1.context.inner.vtable != f2.context.inner.vtable + // : f1.vtable != f2.vtable) { + // return false; // } + // + // if () { + // if (typesEqual(f1.context, f2.context)) { + // f1 = f1.context.inner; + // f2 = f2.context.inner; + // if (identical(f1, f2)) return true; + // goto outerClosureContext; + // } + // return false; + // } + // + // outerClosureContext: + // if (f1.context is #Top && f2.context is #Top) { + // return identical(f1.context, f2.context); + // } + // // return false; // } @@ -1643,21 +1648,58 @@ class Intrinsifier { function, function.locals[1].type, closureBaseStructRef); b.local_set(fun2); - // Compare vtable references + // Compare vtable references. For instantiation closures compare the + // inner vtables + final instantiationContextBase = w.RefType( + translator.closureLayouter.instantiationContextBaseStruct, + nullable: false); + final vtableRefType = w.RefType.def( + translator.closureLayouter.vtableBaseStruct, + nullable: false); + // Returns vtables of closures that we compare for equality. + final vtablesBlock = b.block([], [vtableRefType, vtableRefType]); + // `br` target when fun1 is not an instantiation + final fun1NotInstantiationBlock = + b.block([], [w.RefType.struct(nullable: false)]); + // `br` target when fun1 is an instantiation, but fun2 is not + final fun1InstantiationFun2NotInstantiationBlock = + b.block([], [w.RefType.struct(nullable: false)]); b.local_get(fun1); + b.struct_get(closureBaseStruct, FieldIndex.closureContext); + b.br_on_cast_fail(instantiationContextBase, fun1NotInstantiationBlock); + b.struct_get(translator.closureLayouter.instantiationContextBaseStruct, + FieldIndex.instantiationContextInner); b.struct_get(closureBaseStruct, FieldIndex.closureVtable); b.local_get(fun2); + b.struct_get(closureBaseStruct, FieldIndex.closureContext); + b.br_on_cast_fail( + instantiationContextBase, fun1InstantiationFun2NotInstantiationBlock); + b.struct_get(translator.closureLayouter.instantiationContextBaseStruct, + FieldIndex.instantiationContextInner); b.struct_get(closureBaseStruct, FieldIndex.closureVtable); + b.br(vtablesBlock); + b.end(); // fun1InstantiationFun2NotInstantiationBlock + b.i32_const(0); // false + b.return_(); + b.end(); // fun1NotInstantiationBlock + b.drop(); + b.local_get(fun1); + b.struct_get(closureBaseStruct, FieldIndex.closureVtable); + // To keep the generated code small and simple, instead of checking that + // fun2 is also not an instantiation, we can just return the outer + // (potentially instantiation) vtable here. In the rest of the code + // `ref.eq` will be `false` (as vtable of an instantiation and + // non-instantiation will never be equal) and the function will return + // `false` as expected. + b.local_get(fun2); + b.struct_get(closureBaseStruct, FieldIndex.closureVtable); + b.end(); // vtablesBlock b.ref_eq(); b.if_(); // fun1.vtable == fun2.vtable // Check if closures are instantiations. Since they have the same vtable // it's enough to check just one of them. - final instantiationContextBase = w.RefType( - translator.closureLayouter.instantiationContextBaseStruct, - nullable: false); - final instantiationCheckPassedBlock = b.block(); final notInstantiationBlock = @@ -1669,10 +1711,6 @@ class Intrinsifier { // Closures are instantiations. Compare inner function vtables to check // that instantiations are for the same generic function. - final vtable1Local = codeGen.function.addLocal(w.RefType.def( - translator.closureLayouter.vtableBaseStruct, - nullable: false)); - void getInstantiationContextInner(w.Local fun) { b.local_get(fun); // instantiation.context @@ -1683,24 +1721,15 @@ class Intrinsifier { FieldIndex.instantiationContextInner); } - getInstantiationContextInner(fun1); - b.struct_get(closureBaseStruct, FieldIndex.closureVtable); - b.local_tee(vtable1Local); - - getInstantiationContextInner(fun2); - b.struct_get(closureBaseStruct, FieldIndex.closureVtable); - - b.ref_eq(); - b.if_(); // fun1.context.inner.vtable == fun2.context.inner.vtable - - // Closures are instantiations of the same function, compare types + // Closures are instantiations of the same function, compare types. b.local_get(fun1); b.struct_get(closureBaseStruct, FieldIndex.closureContext); b.ref_cast(instantiationContextBase); b.local_get(fun2); b.struct_get(closureBaseStruct, FieldIndex.closureContext); b.ref_cast(instantiationContextBase); - b.local_get(vtable1Local); + getInstantiationContextInner(fun1); + b.struct_get(closureBaseStruct, FieldIndex.closureVtable); b.ref_cast(w.RefType.def( translator.closureLayouter.genericVtableBaseStruct, nullable: false)); @@ -1722,8 +1751,6 @@ class Intrinsifier { b.end(); b.i32_const(0); // false b.return_(); - b.end(); // fun1.context.inner.vtable == fun2.context.inner.vtable - // Contexts or inner vtables are not equal b.i32_const(0); // false b.return_(); b.end(); // notInstantiationBlock