diff --git a/tslang/lib/TypeScript/MLIRGenImpl.h b/tslang/lib/TypeScript/MLIRGenImpl.h index faade2dbf..da33f0c86 100644 --- a/tslang/lib/TypeScript/MLIRGenImpl.h +++ b/tslang/lib/TypeScript/MLIRGenImpl.h @@ -1475,6 +1475,70 @@ class MLIRGenImpl return varOp && varOp->hasAttr(OWNED_LOCAL_ATTR_NAME); } + // A record read out whole from the temporary slot an object literal is built in (one that + // owns nothing of its own), with a field given a value read out of another object's field + // or element, or out of an owning local: what `{ s: p.child }` and `{ x: a }` are. Such a + // value carries no reference, and the literal takes none for it. + static bool readsLiteralSlot(MLIRTypeHelper &mth, mlir::Location location, mlir::Value value) + { + auto loadOp = value ? value.getDefiningOp() : mlir_ts::LoadOp(); + auto varOp = loadOp ? loadOp.getReference().getDefiningOp() : mlir_ts::VariableOp(); + if (!varOp || varOp->hasAttr(OWNED_LOCAL_ATTR_NAME)) + { + return false; + } + + for (auto *user : varOp->getUsers()) + { + auto fieldRef = dyn_cast(user); + if (!fieldRef) + { + continue; + } + + for (auto *fieldUser : fieldRef->getUsers()) + { + auto storeOp = dyn_cast(fieldUser); + if (!storeOp || storeOp.getReference() != fieldRef.getResult()) + { + continue; + } + + // only a field that owns a block counts: not a reference (a generator's + // `.captured`), and not a method bound to the object it was read from (`{ ...it }` + // of a generator), whose routines do not pair up for it + auto storedType = storeOp.getValue().getType(); + if (!mth.ownsHeapMemory(location, storedType) || + isa(storedType)) + { + continue; + } + + auto stored = storeOp.getValue(); + while (auto castOp = stored.getDefiningOp()) + { + stored = castOp.getIn(); + } + + auto storedLoad = stored.getDefiningOp(); + auto *place = storedLoad ? storedLoad.getReference().getDefiningOp() : nullptr; + if (place && isa(place)) + { + return true; + } + + // a local that owns what it holds (`{ x: a }`, #454's shape), or a literal + // nested in this one + if (readsLocalSlot(stored) || readsLiteralSlot(mth, location, stored)) + { + return true; + } + } + } + + return false; + } + mlir::LogicalResult processConstRef(mlir::Location location, mlir::OpBuilder &builder, const GenContext &genContext) { if (mlir::failed(getVariableTypeAndInit(location, genContext))) @@ -1518,10 +1582,17 @@ class MLIRGenImpl // (`let a = new C(); const b = a;`, a parameter too): folded, it would be a read of // that slot, holding no reference of its own, and `a = d` would free what `b` still // reads (#454). A const of a const, a call or a field has no slot to lose it from. + // + // And a const record whose fields own blocks (`const o = { s: p.child }`): the literal + // hands its fields over unretained - its receiver is the one that retains them, as a + // `let` does - so folded, `o` held none of them, and `p.child = d` freed what `o.s` + // still read (#460). MLIRTypeHelper mth(builder.getContext(), compileOptions); needsIdentityStorage = mth.hasBoundMethodField(type) || isa(type) || ((isa(type) || MLIRTypeHelper::isSharedHandleType(type) - || (readsLocalSlot(initial) && mth.ownsHeapMemory(location, type))) + || (readsLocalSlot(initial) && mth.ownsHeapMemory(location, type)) + || (readsLiteralSlot(mth, location, initial) && isa(type) + && mth.ownsHeapMemory(location, type))) && !(varClass == VariableType::ConstRef) && !(initial && initial.getDefiningOp())); if (needsIdentityStorage) diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 0f53f727a..48f3b2923 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -471,6 +471,7 @@ tslang_add_test(NAME test-compile-00-global-object-literal COMMAND test-runner " tslang_add_test(NAME test-compile-00-instanceof COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00instanceof.ts") tslang_add_test(NAME test-compile-00-const-of-let COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_of_let.ts") tslang_add_test(NAME test-compile-00-instanceof-union COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00instanceof_union.ts") +tslang_add_test(NAME test-compile-00-const-object-literal-field COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_object_literal_field.ts") tslang_add_test(NAME test-compile-00-nullable-to-any COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00nullable_to_any.ts") tslang_add_test(NAME test-compile-00-ternary-new-or-null COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00ternary_new_or_null.ts") tslang_add_test(NAME test-compile-00-class COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class.ts") @@ -959,6 +960,7 @@ tslang_add_test(NAME test-jit-00-global-object-literal COMMAND test-runner -jit tslang_add_test(NAME test-jit-00-instanceof COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00instanceof.ts") tslang_add_test(NAME test-jit-00-const-of-let COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_of_let.ts") tslang_add_test(NAME test-jit-00-instanceof-union COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00instanceof_union.ts") +tslang_add_test(NAME test-jit-00-const-object-literal-field COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_object_literal_field.ts") tslang_add_test(NAME test-jit-00-nullable-to-any COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00nullable_to_any.ts") tslang_add_test(NAME test-jit-00-ternary-new-or-null COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00ternary_new_or_null.ts") tslang_add_test(NAME test-jit-00-class COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class.ts") @@ -1775,6 +1777,7 @@ set(TSLANG_CORPUS 00in.ts 00global_object_literal.ts 00infer.ts + 00const_object_literal_field.ts 00global_union_from_call.ts 00instanceof.ts 00const_of_let.ts diff --git a/tslang/test/tester/tests/00const_object_literal_field.ts b/tslang/test/tester/tests/00const_object_literal_field.ts new file mode 100644 index 000000000..b22b64c19 --- /dev/null +++ b/tslang/test/tester/tests/00const_object_literal_field.ts @@ -0,0 +1,49 @@ +// A `const` object literal keeps what its fields hold (#460). Under rc the const was folded into a +// read of the literal's temporary slot, which took no reference for a field read out of another +// object (`{ s: p.child }`), so overwriting `p.child` freed what the literal still held. Each field +// is read after allocations of the same size have reused anything freed. +class Node { + v = 0; + name = ""; +} + +class P { + child: Node = new Node(); +} + +function churn() { + let keep: Node[] = []; + for (let i = 0; i < 1000; i++) { + const n = new Node(); + n.v = -i; + n.name = "junk" + i; + keep.push(n); + } + + return keep.length; +} + +function main() { + const p = new P(); + p.child.name = "first" + 1; + const o = { s: p.child, n: 2 }; + p.child = new Node(); + assert(churn() == 1000); + assert(o.s.name == "first1" && o.n == 2, "a const literal keeps the field it read"); + + let a = new Node(); + a.name = "local" + 3; + const t = { x: a, inner: { y: a } }; + a = new Node(); + assert(churn() == 1000); + assert(t.x.name == "local3" && t.inner.y.name == "local3", "a const literal keeps a local it read"); + + for (let i = 0; i < 1000; i++) { + const q = new P(); + const held = { c: q.child }; + q.child = new Node(); + held.c.v = i; + } + + print("done."); +}