diff --git a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp index 797663804..a371bdc87 100644 --- a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp +++ b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp @@ -17,6 +17,9 @@ #include "llvm/ADT/TypeSwitch.h" #include "llvm/Support/Debug.h" +#include +#include + #define DEBUG_TYPE "pass" namespace mlir_ts = mlir::typescript; @@ -1361,6 +1364,7 @@ class OwnedReturnConsumptionPass }); mlir::OpBuilder builder(&getContext()); + std::set> placedAfterFinally; for (auto *op : discarded) { if (!allUsesReleasableInOwnBlock(op) || storedIntoUnownedLocalOutside(op)) @@ -1414,7 +1418,7 @@ class OwnedReturnConsumptionPass auto leavesBlock = mlir::isa(nested) || (mlir::isa(nested) && jumpLeavesBlock(nested, block)); - if (leavesBlock) + if (leavesBlock && !releaseAfterFinally(builder, op, nested, block, placedAfterFinally)) { builder.setInsertionPoint(nested); builder.create(op->getLoc(), op->getResult(0)); @@ -1446,6 +1450,85 @@ class OwnedReturnConsumptionPass } } + // A jump out of a `try` body or catch clause runs the `finally` first, and the finally may read + // the temporary: `const v = mv(); try { if (c) continue; } finally { s += v.get(); }` gave `v` + // back at the `continue`, before the finally read it. Or the finally may leave by a jump of its + // own, which releases the temporary in front of it too: `try { continue; } finally { break; }` + // gave it back twice. When a jump passes through a finally between it and the temporary's block, + // the release goes at the end of the outermost such finally instead - the last one the jump + // runs - inside a `ts.If` tagged with the jump, as MLIRGen does for the scopes the jump leaves + // (mlirGenScopeExitAfterFinally), so that TryOpLowering keeps it in the copy of the finally that + // jump runs and in no other. A finally that leaves on its own never reaches it. False when the + // jump passes through no finally. + // + // Every jump with one key runs one copy, so the release goes in once for each (`placed`). + static bool releaseAfterFinally(mlir::OpBuilder &builder, mlir::Operation *op, mlir::Operation *jump, + mlir::Block *block, + std::set> &placed) + { + mlir_ts::TryOp lastTry; + for (auto *child = jump; child->getBlock() != block; child = child->getParentOp()) + { + auto tryOp = mlir::dyn_cast(child->getParentOp()); + if (!tryOp || child->getParentRegion() == &tryOp.getFinally() || + child->getParentRegion() == &tryOp.getCleanup()) + { + continue; + } + + auto &finally = tryOp.getFinally(); + if (finally.empty() || &finally.front().front() == finally.front().getTerminator()) + { + continue; + } + + lastTry = tryOp; + } + + if (!lastTry) + { + return false; + } + + auto value = op->getResult(0); + + auto isReturn = mlir::isa(jump); + std::string key; + if (!isReturn) + { + auto isBreak = mlir::isa(jump); + auto label = isBreak ? mlir::cast(jump).getLabelAttr() + : mlir::cast(jump).getLabelAttr(); + key = std::string(isBreak ? JUMP_ONLY_BREAK_KEY : JUMP_ONLY_CONTINUE_KEY) + + (label ? label.getValue().str() : std::string()); + } + + if (!placed.insert({op, lastTry.getOperation(), isReturn ? std::string(RETURN_ONLY_ATTR_NAME) : key}).second) + { + return true; + } + + auto location = op->getLoc(); + auto &finallyEnd = lastTry.getFinally().back(); + builder.setInsertionPoint(finallyEnd.getTerminator()); + auto alwaysTrue = builder.create(location, mlir_ts::BooleanType::get(builder.getContext()), + builder.getBoolAttr(true)); + auto ifOp = builder.create(location, alwaysTrue, /*withElseRegion=*/false); + if (isReturn) + { + ifOp->setAttr(RETURN_ONLY_ATTR_NAME, builder.getUnitAttr()); + } + else + { + ifOp->setAttr(JUMP_ONLY_ATTR_NAME, builder.getStringAttr(key)); + } + + mlir::OpBuilder::InsertionGuard guard(builder); + builder.setInsertionPointToStart(&ifOp.getThenRegion().front()); + builder.create(location, value); + return true; + } + // Does `op` catch this jump - is it the loop, switch or labelled statement the jump names? // // An unlabelled `break` is caught by the nearest enclosing loop or `switch`; an unlabelled diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 7f76feed5..fa2f9b7b2 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -405,6 +405,7 @@ tslang_add_test(NAME test-compile-00-const-element-read COMMAND test-runner "${P tslang_add_test(NAME test-compile-00-const-global-read COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_global_read.ts") tslang_add_test(NAME test-compile-00-global-argument-reassigned COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_argument_reassigned.ts") tslang_add_test(NAME test-compile-00-global-argument-view COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_argument_view.ts") +tslang_add_test(NAME test-compile-00-temporary-read-in-finally COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00temporary_read_in_finally.ts") tslang_add_test(NAME test-compile-00-unowned-local-store COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00unowned_local_store.ts") tslang_add_test(NAME test-compile-00-array-argument-held COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_argument_held.ts") tslang_add_test(NAME test-compile-00-array-data-string-alias COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_data_string_alias.ts") @@ -986,6 +987,7 @@ tslang_add_test(NAME test-jit-00-const-element-read COMMAND test-runner -jit "${ tslang_add_test(NAME test-jit-00-const-global-read COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_global_read.ts") tslang_add_test(NAME test-jit-00-global-argument-reassigned COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_argument_reassigned.ts") tslang_add_test(NAME test-jit-00-global-argument-view COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_argument_view.ts") +tslang_add_test(NAME test-jit-00-temporary-read-in-finally COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00temporary_read_in_finally.ts") tslang_add_test(NAME test-jit-00-unowned-local-store COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00unowned_local_store.ts") tslang_add_test(NAME test-jit-00-array-argument-held COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_argument_held.ts") tslang_add_test(NAME test-jit-00-array-data-string-alias COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_data_string_alias.ts") @@ -1957,6 +1959,7 @@ set(TSLANG_CORPUS 00const_global_read.ts 00global_argument_reassigned.ts 00global_argument_view.ts + 00temporary_read_in_finally.ts 00unowned_local_store.ts 00array_argument_held.ts 00array_data_string_alias.ts diff --git a/tslang/test/tester/tests/00temporary_read_in_finally.ts b/tslang/test/tester/tests/00temporary_read_in_finally.ts new file mode 100644 index 000000000..5c4c13732 --- /dev/null +++ b/tslang/test/tester/tests/00temporary_read_in_finally.ts @@ -0,0 +1,177 @@ +// A `const` holding a call's result (a folded const, released where its block ends) stays alive +// when a `break`, `continue` or `return` leaves its block through a `try` whose `finally` reads +// it, or leaves by a jump of its own. Under rc the release went in front of the jump, and the +// finally then ran on a freed block, or released it a second time in front of its own jump. +// Each read follows allocations of the same size, so a freed block is reused. +class V { + tag = ""; + constructor(public x: number) { + this.tag = "v" + x; + } + + get() { + return this.x; + } +} + +function mv(x: number) { + return new V(x); +} + +function churn() { + const junk: V[] = []; + for (let i = 0; i < 64; i++) { + junk.push(new V(999)); + } +} + +function viaContinue() { + let s: number = 0; + for (let i = 0; i < 4; i++) { + const v = mv(i); + try { + churn(); + if (i % 2 == 0) { + continue; + } + } finally { + churn(); + s += v.get(); + } + } + + return s; +} + +function viaBreak() { + let s: number = 0; + for (let i = 0; i < 4; i++) { + const v = mv(i + 10); + try { + if (i == 1) { + break; + } + } finally { + churn(); + s += v.x; + } + } + + return s; +} + +function viaTwoJumps() { + let s: number = 0; + for (let i = 0; i < 6; i++) { + const v = mv(i); + try { + if (i % 3 == 0) { + continue; + } + + if (i % 3 == 1) { + continue; + } + } finally { + churn(); + s += v.get(); + } + } + + return s; +} + +function viaLabelledContinue() { + let s: number = 0; + outer: for (let i = 0; i < 3; i++) { + const v = mv(i + 1); + for (let j = 0; j < 3; j++) { + try { + try { + if (j == 1) { + continue outer; + } + } finally { + churn(); + s += v.get(); + } + } finally { + churn(); + s += v.x * 100; + } + } + } + + return s; +} + +function viaReturn(x: number) { + const v = mv(x); + try { + if (x > 0) { + return x; + } + } finally { + churn(); + assert(v.x == x && v.tag == "v" + x, "read in the finally of a return"); + } + + return -1; +} + +function jumpInFinally() { + let s: number = 0; + for (let i = 0; i < 4; i++) { + const v = mv(i); + try { + try { + if (i % 2 == 0) { + continue; + } + } finally { + churn(); + if (i == 2) { + break; + } + } + } finally { + } + + s += v.x; + } + + churn(); + return s; +} + +function callInFinally(v: V) { + churn(); + return v.get(); +} + +function viaCallInFinally() { + let s: number = 0; + for (let i = 0; i < 3; i++) { + const v = mv(i + 5); + try { + if (v.x > 2) { + continue; + } + } finally { + s += callInFinally(v); + } + } + + return s; +} + +function main() { + assert(viaContinue() == 6, "continue through a finally"); + assert(viaBreak() == 21, "break through a finally"); + assert(viaTwoJumps() == 15, "two continues through one finally"); + assert(viaLabelledContinue() == 1212, "a labelled continue through two finallys"); + assert(viaReturn(3) == 3, "return through a finally"); + assert(viaCallInFinally() == 18, "a call in the finally"); + assert(jumpInFinally() == 1, "a continue through a finally that breaks"); + print("done."); +}