rc: hold an argument whose place the call overwrites

tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp

@@ -604,6 +604,18 @@ class OwnedReturnConsumptionPass
                     continue;
                 }
 
+                // An argument read out of a slot or a place that a later argument of the same call
+                // overwrites (`f(n, (n = mk(), n))`), or out of a field or an element of an object
+                // the call is given as well (`f(n, n.items[0])`, which `f` may overwrite through
+                // its other parameter): either one frees the block the argument read. Under rc the
+                // caller holds it, as it holds a global's (A1, R8 of the fuzzer).
+                if (compileOptions.isRefCounted() &&
+                    (overwrittenBeforeCall(argument, op) || readFromOtherArgument(argument, op)))
+                {
+                    held.push_back({op, argument});
+                    continue;
+                }
+
                 auto global = globalOf(argument, false);
                 auto isHandle = MLIRTypeHelper::isSharedHandleType(argument.getType());
                 if (global.empty() || (isHandle ? !mayAssign(op, global) : !compileOptions.isRefCounted()))
@@ -688,6 +700,126 @@ class OwnedReturnConsumptionPass
         }
     }
 
+    static mlir::Value stripCasts(mlir::Value value)
+    {
+        while (auto castOp = value.getDefiningOp<mlir_ts::CastOp>())
+        {
+            value = castOp.getIn();
+        }
+
+        return value;
+    }
+
+    // Do two references name the same place? One SSA reference, one global, or one field of the
+    // same value - each access to a field builds a `ts.PropertyRef` of its own.
+    static bool samePlace(mlir::Value a, mlir::Value b)
+    {
+        if (a == b)
+        {
+            return true;
+        }
+
+        auto globalA = a.getDefiningOp<mlir_ts::AddressOfOp>();
+        auto globalB = b.getDefiningOp<mlir_ts::AddressOfOp>();
+        if (globalA && globalB)
+        {
+            return globalA.getGlobalName() == globalB.getGlobalName();
+        }
+
+        auto fieldA = a.getDefiningOp<mlir_ts::PropertyRefOp>();
+        auto fieldB = b.getDefiningOp<mlir_ts::PropertyRefOp>();
+        return fieldA && fieldB && fieldA.getPosition() == fieldB.getPosition() &&
+               sameValue(fieldA->getOperand(0), fieldB->getOperand(0));
+    }
+
+    // Are two values one value: one SSA value, or reads of one place?
+    static bool sameValue(mlir::Value a, mlir::Value b)
+    {
+        a = stripCasts(a);
+        b = stripCasts(b);
+        if (a == b)
+        {
+            return true;
+        }
+
+        auto loadA = a.getDefiningOp<mlir_ts::LoadOp>();
+        auto loadB = b.getDefiningOp<mlir_ts::LoadOp>();
+        return loadA && loadB && samePlace(loadA.getReference(), loadB.getReference());
+    }
+
+    // Is the argument read out of a slot or a place that something between the read and the call
+    // overwrites - a later argument (`f(n, (n = mk(), n))`)?
+    //
+    // So is one read out of a field or an element of a value read so: overwriting `n` in
+    // `f({ v: n.v }, (n = mk(), 0))` gives back the object whose field the argument read.
+    static bool overwrittenBeforeCall(mlir::Value argument, mlir::Operation *call)
+    {
+        auto value = stripCasts(argument);
+        for (auto depth = 0; depth < 8; depth++)
+        {
+            auto loadOp = value.getDefiningOp<mlir_ts::LoadOp>();
+            if (!loadOp || loadOp->getBlock() != call->getBlock())
+            {
+                return false;
+            }
+
+            auto reference = loadOp.getReference();
+            for (auto *op = loadOp->getNextNode(); op && op != call; op = op->getNextNode())
+            {
+                auto overwrites = op->walk([&](mlir::Operation *nested) {
+                    auto storeOp = mlir::dyn_cast<mlir_ts::StoreOp>(nested);
+                    auto releaseSlotOp = mlir::dyn_cast<mlir_ts::ReleaseSlotOp>(nested);
+                    auto place =
+                        storeOp ? storeOp.getReference() : releaseSlotOp ? releaseSlotOp.getSlot() : mlir::Value();
+                    return place && samePlace(place, reference) ? mlir::WalkResult::interrupt()
+                                                                : mlir::WalkResult::advance();
+                });
+
+                if (overwrites.wasInterrupted())
+                {
+                    return true;
+                }
+            }
+
+            auto *place = reference.getDefiningOp();
+            if (!mlir::isa_and_nonnull<mlir_ts::PropertyRefOp, mlir_ts::ElementRefOp, mlir_ts::InterfaceSymbolRefOp>(place))
+            {
+                return false;
+            }
+
+            value = stripCasts(place->getOperand(0));
+        }
+
+        return false;
+    }
+
+    // Is the argument read out of a field or an element of a value the call is given as another
+    // argument (`f(n, n.items[0])`)?
+    static bool readFromOtherArgument(mlir::Value argument, mlir::Operation *call)
+    {
+        auto value = stripCasts(argument);
+        for (auto depth = 0; depth < 8; depth++)
+        {
+            auto loadOp = value.getDefiningOp<mlir_ts::LoadOp>();
+            auto *place = loadOp ? loadOp.getReference().getDefiningOp() : nullptr;
+            if (!mlir::isa_and_nonnull<mlir_ts::PropertyRefOp, mlir_ts::ElementRefOp, mlir_ts::InterfaceSymbolRefOp>(place))
+            {
+                return false;
+            }
+
+            value = place->getOperand(0);
+            for (auto other : own_facts::callArgs(call))
+            {
+                if (stripCasts(other) != stripCasts(argument) && sameValue(other, value))
+                {
+                    return true;
+                }
+            }
+        }
+
+        return false;
+    }
+
     // Where an exception out of `op` is taken first within its function: the region of the try
     // around it that runs then, in the order its landing pad takes them (TryOpLowering) - the
     // cleanup, else the catch clause, else the finally, for a call in its body; the finally for a

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-argument-place-overwritten COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00argument_place_overwritten.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")
@@ -987,6 +988,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-argument-place-overwritten COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00argument_place_overwritten.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")
@@ -1959,6 +1961,7 @@ set(TSLANG_CORPUS
     00const_global_read.ts
     00global_argument_reassigned.ts
     00global_argument_view.ts
+    00argument_place_overwritten.ts
     00temporary_read_in_finally.ts
     00unowned_local_store.ts
     00array_argument_held.ts

tslang/test/tester/tests/00argument_place_overwritten.ts

@@ -0,0 +1,122 @@
+// An argument read out of a local, a parameter or a field stays alive when the call overwrites
+// where it was read: a later argument assigns it (`check(n, (n = mk(2), n), 1)`) or the object it
+// was read out of (`checkItem(n.items[0], (n = mk(2), 0), 101)`), or the callee overwrites a field
+// or an element of an object it is given together with what was read out of it
+// (`overwrite(n, n.items[0])`). The argument is borrowed, so under rc the overwrite freed the block
+// the parameter still read. 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;
+    }
+}
+
+class N {
+    v: V;
+    items: V[];
+    constructor(x: number) {
+        this.v = new V(x);
+        this.items = [new V(x + 100)];
+    }
+}
+
+function mk(x: number) {
+    return new N(x);
+}
+
+function churn() {
+    const junk: N[] = [];
+    for (let i = 0; i < 64; i++) {
+        junk.push(new N(999));
+    }
+}
+
+function check(n: N, other: N, x: number) {
+    churn();
+    return n.v.x == x && n.v.tag == "v" + x && other.v.x > 0;
+}
+
+function localReassigned() {
+    let n = mk(1);
+    return check(n, (n = mk(2), n), 1) && n.v.x == 2;
+}
+
+function localReassignedInLoop() {
+    let n = mk(0);
+    for (let i = 1; i < 4; i++) {
+        if (!check(n, (n = mk(i), n), i - 1)) {
+            return false;
+        }
+    }
+
+    return n.v.x == 3;
+}
+
+function parameterReassigned(p: N) {
+    return check(p, (p = mk(7), p), 6);
+}
+
+class H {
+    n = mk(0);
+}
+
+function fieldReassigned() {
+    const h = new H();
+    h.n = mk(3);
+    return check(h.n, (h.n = mk(4), h.n), 3) && h.n.v.x == 4;
+}
+
+interface IV {
+    v: V;
+}
+
+function checkView(iv: IV, n: number, x: number) {
+    churn();
+    return iv.v.x == x && iv.v.tag == "v" + x;
+}
+
+function checkItem(v: V, n: number, x: number) {
+    churn();
+    return v.x == x && v.tag == "v" + x;
+}
+
+function localsFieldInLiteral() {
+    let n = mk(1);
+    return checkView({ v: n.v }, (n = mk(2), 0), 1);
+}
+
+function localsElement() {
+    let n = mk(1);
+    return checkItem(n.items[0], (n = mk(2), 0), 101);
+}
+
+function overwriteElement(n: N, item: V) {
+    n.items[0] = new V(5);
+    churn();
+    return item.x == 108 && item.tag == "v108";
+}
+
+function overwriteField(n: N, v: V) {
+    n.v = new V(5);
+    churn();
+    return v.x == 9 && v.tag == "v9";
+}
+
+function main() {
+    for (let i = 0; i < 3; i++) {
+        assert(localReassigned(), "a local given before an argument that assigns it");
+        assert(localReassignedInLoop(), "a local given before an argument that assigns it, in a loop");
+        assert(parameterReassigned(mk(6)), "a parameter given before an argument that assigns it");
+        assert(fieldReassigned(), "a local's field given before an argument that assigns it");
+        assert(localsFieldInLiteral(), "a field of a local, in an object literal, before an argument that assigns the local");
+        assert(localsElement(), "an element of a local given before an argument that assigns the local");
+
+        let n = mk(8);
+        assert(overwriteElement(n, n.items[0]), "an element given with its object, overwritten through it");
+        n = mk(9);
+        assert(overwriteField(n, n.v), "a field given with its object, overwritten through it");
+    }
+
+    print("done.");
+}