From 57d5c1465a576e2d8bfc6496946aa24183c4456d Mon Sep 17 00:00:00 2001 From: Clyybber Date: Thu, 21 Jan 2021 15:25:00 +0100 Subject: [PATCH] Part-to-whole optimization (#16775) --- compiler/injectdestructors.nim | 31 +++++++++++----------- tests/arc/tmovebug.nim | 47 +++++++++++++++++++++++++++++++++- 2 files changed, 62 insertions(+), 16 deletions(-) diff --git a/compiler/injectdestructors.nim b/compiler/injectdestructors.nim index 48c501209..fc404f90f 100644 --- a/compiler/injectdestructors.nim +++ b/compiler/injectdestructors.nim @@ -21,16 +21,6 @@ import from trees import exprStructuralEquivalent, getRoot -type - Scope = object # well we do scope-based memory management. \ - # a scope is comparable to an nkStmtListExpr like - # (try: statements; dest = y(); finally: destructors(); dest) - vars: seq[PSym] - wasMoved: seq[PNode] - final: seq[PNode] # finally section - needsTry: bool - parent: ptr Scope - type Con = object owner: PSym @@ -42,6 +32,15 @@ type uninitComputed: bool idgen: IdGenerator + Scope = object # we do scope-based memory management. + # a scope is comparable to an nkStmtListExpr like + # (try: statements; dest = y(); finally: destructors(); dest) + vars: seq[PSym] + wasMoved: seq[PNode] + final: seq[PNode] # finally section + needsTry: bool + parent: ptr Scope + ProcessMode = enum normal consumed @@ -1006,11 +1005,13 @@ proc moveOrCopy(dest, ri: PNode; c: var Con; s: var Scope, isDecl = false): PNod if isUnpackedTuple(ri[0]): # unpacking of tuple: take over the elements result = c.genSink(dest, p(ri, c, s, consumed), isDecl) - elif isAnalysableFieldAccess(ri, c.owner) and isLastRead(ri, c) and - not aliases(dest, ri): - # Rule 3: `=sink`(x, z); wasMoved(z) - var snk = c.genSink(dest, ri, isDecl) - result = newTree(nkStmtList, snk, c.genWasMoved(ri)) + elif isAnalysableFieldAccess(ri, c.owner) and isLastRead(ri, c): + if not aliases(dest, ri): + # Rule 3: `=sink`(x, z); wasMoved(z) + var snk = c.genSink(dest, ri, isDecl) + result = newTree(nkStmtList, snk, c.genWasMoved(ri)) + else: + result = c.genSink(dest, destructiveMoveVar(ri, c, s), isDecl) else: result = c.genCopy(dest, ri) result.add p(ri, c, s, consumed) diff --git a/tests/arc/tmovebug.nim b/tests/arc/tmovebug.nim index d43b3489e..71962e21d 100644 --- a/tests/arc/tmovebug.nim +++ b/tests/arc/tmovebug.nim @@ -92,6 +92,12 @@ copy destroy destroy destroy +part-to-whole assigment: +sink +(children: @[]) +destroy +copy +destroy ''' """ @@ -578,7 +584,7 @@ let w3 = newWrapper2(-1) echo $w3[] -#--------------------------------------------------------------------- +#-------------------------------------------------------------------- #self-assignments # Self-assignments that are not statically determinable will get @@ -668,3 +674,42 @@ proc caseNotAConstant = echo s # @[(f: 2), (f: 2), (f: 3)] caseNotAConstant() + + +#-------------------------------------------------------------------- +echo "part-to-whole assigment:" + +type + Tree = object + children: seq[Tree] + + TreeDefaultHooks = object + children: seq[TreeDefaultHooks] + +proc `=destroy`(x: var Tree) = echo "destroy" +proc `=sink`(x: var Tree, y: Tree) = echo "sink" +proc `=copy`(x: var Tree, y: Tree) = echo "copy" + +proc partToWholeSeq = + var t = Tree(children: @[Tree()]) + t = t.children[0] # This should be sunk, but with the special transform (tmp = t.children[0]; wasMoved(0); `=sink`(t, tmp)) + + var tc = TreeDefaultHooks(children: @[TreeDefaultHooks()]) + tc = tc.children[0] # Ditto; if this were sunk with the normal transform (`=sink`(t, t.children[0]); wasMoved(t.children[0])) + echo tc # then it would crash because t.children[0] does not exist after the call to `=sink` + +partToWholeSeq() + +type List = object + next: ref List + +proc `=destroy`(x: var List) = echo "destroy" +proc `=sink`(x: var List, y: List) = echo "sink" +proc `=copy`(x: var List, y: List) = echo "copy" + +proc partToWholeUnownedRef = + var t = List(next: new List) + t = t.next[] # Copy because t.next is not an owned ref, and thus t.next[] cannot be moved + +partToWholeUnownedRef() +