From 5f5879dc4cb018407ea69b50ca3fff600dfb8949 Mon Sep 17 00:00:00 2001 From: Clyybber Date: Tue, 1 Oct 2019 14:09:24 +0200 Subject: [PATCH] Refactor injectdestructors (#12295) One improvement over #devel is visible in the transformation of getEnv. With this approach we move to result whenever possible. --- compiler/ast.nim | 4 - compiler/cursors.nim | 72 ---- compiler/injectdestructors.nim | 737 ++++++++++++--------------------- compiler/pragmas.nim | 7 +- compiler/transf.nim | 1 - compiler/wordrecg.nim | 4 - doc/nimc.rst | 9 - 7 files changed, 257 insertions(+), 577 deletions(-) delete mode 100644 compiler/cursors.nim diff --git a/compiler/ast.nim b/compiler/ast.nim index a2598dae44..f24008b305 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -291,10 +291,6 @@ type const sfNoInit* = sfMainModule # don't generate code to init the variable - sfCursor* = sfDispatcher - # local variable has been computed to be a "cursor". - # see cursors.nim for details about what that means. - sfAllUntyped* = sfVolatile # macro or template is immediately expanded \ # in a generic context diff --git a/compiler/cursors.nim b/compiler/cursors.nim deleted file mode 100644 index 9577102fb8..0000000000 --- a/compiler/cursors.nim +++ /dev/null @@ -1,72 +0,0 @@ -# -# -# The Nim Compiler -# (c) Copyright 2019 Andreas Rumpf -# -# See the file "copying.txt", included in this -# distribution, for details about the copyright. -# - -import - intsets, ast, astalgo, msgs, renderer, magicsys, types, idents, trees, - strutils, options, dfa, lowerings, tables, modulegraphs, msgs, - lineinfos, parampatterns - -##[ -This module implements "cursor" detection. A cursor is a local variable -that is used for navigation in a datastructure, it does not "own" the -data it aliases but it might update the underlying datastructure. - -Two primary examples for cursors that I have in mind and that are critical -for optimization: - -1. Local string variable introduced by ``for x in a``:: - - var i = 0 - while i < a.len: - let cursor = a[i] - use cursor - inc i - -2. Local ``ref`` variable for navigation:: - - var cursor = listHead - while cursor != nil: - use cursor - cursor = cursor.next - -Cursors are very interesting for the optimizer because they can be copyMem'ed -and don't need a destructor. - -More formally, a cursor is a variable that is set on all paths to -a *location* or a proc call that produced a ``lent/var`` type. All statements -that come after these assignments MUST not mutate what the cursor aliases. - -Mutations *through* the cursor are allowed if the cursor has ref semantics. - -Look at this complex real world example taken from the compiler itself: - -.. code-block:: Nim - - proc getTypeName(m: BModule; typ: PType; sig: SigHash): Rope = - var t = typ - while true: - if t.sym != nil and {sfImportc, sfExportc} * t.sym.flags != {}: - return t.sym.loc.r - - if t.kind in irrelevantForBackend: - t = t.lastSon - else: - break - let typ = if typ.kind in {tyAlias, tySink, tyOwned}: typ.lastSon else: typ - if typ.loc.r == nil: - typ.loc.r = typ.typeName & $sig - result = typ.loc.r - if result == nil: internalError(m.config, "getTypeName: " & $typ.kind) - -Here `t` is a cursor but without a control flow based analysis we are unlikely -to detect it. - -]## - -# Araq: I owe you an implementation. For now use the .cursor pragma. :-/ diff --git a/compiler/injectdestructors.nim b/compiler/injectdestructors.nim index 5354fd7405..95de5777ef 100644 --- a/compiler/injectdestructors.nim +++ b/compiler/injectdestructors.nim @@ -11,136 +11,14 @@ ## an optimizer that optimizes copies to moves. This is implemented as an ## AST to AST transformation so that every backend benefits from it. -## Rules for destructor injections: -## -## foo(bar(X(), Y())) -## X and Y get destroyed after bar completes: -## -## foo( (tmpX = X(); tmpY = Y(); tmpBar = bar(tmpX, tmpY); -## destroy(tmpX); destroy(tmpY); -## tmpBar)) -## destroy(tmpBar) -## -## var x = f() -## body -## -## is the same as: -## -## var x; -## try: -## move(x, f()) -## finally: -## destroy(x) -## -## But this really just an optimization that tries to avoid to -## introduce too many temporaries, the 'destroy' is caused by -## the 'f()' call. No! That is not true for 'result = f()'! -## -## x = y where y is read only once -## is the same as: move(x, y) -## -## Actually the more general rule is: The *last* read of ``y`` -## can become a move if ``y`` is the result of a construction. -## -## We also need to keep in mind here that the number of reads is -## control flow dependent: -## let x = foo() -## while true: -## y = x # only one read, but the 2nd iteration will fail! -## This also affects recursions! Only usages that do not cross -## a loop boundary (scope) and are not used in function calls -## are safe. -## -## -## x = f() is the same as: move(x, f()) -## -## x = y -## is the same as: copy(x, y) -## -## Reassignment works under this scheme: -## var x = f() -## x = y -## -## is the same as: -## -## var x; -## try: -## move(x, f()) -## copy(x, y) -## finally: -## destroy(x) -## -## result = f() must not destroy 'result'! -## -## The produced temporaries clutter up the code and might lead to -## inefficiencies. A better strategy is to collect all the temporaries -## in a single object that we put into a single try-finally that -## surrounds the proc body. This means the code stays quite efficient -## when compiled to C. In fact, we do the same for variables, so -## destructors are called when the proc returns, not at scope exit! -## This makes certains idioms easier to support. (Taking the slice -## of a temporary object.) -## -## foo(bar(X(), Y())) -## X and Y get destroyed after bar completes: -## -## var tmp: object -## foo( (move tmp.x, X(); move tmp.y, Y(); tmp.bar = bar(tmpX, tmpY); -## tmp.bar)) -## destroy(tmp.bar) -## destroy(tmp.x); destroy(tmp.y) -## +## See doc/destructors.rst for a spec of the implemented rewrite rules -#[ -From https://github.com/nim-lang/Nim/wiki/Destructors - -Rule Pattern Transformed into ----- ------- ---------------- -1.1 var x: T; stmts var x: T; try stmts - finally: `=destroy`(x) -2 x = f() `=sink`(x, f()) -3 x = lastReadOf z `=sink`(x, z); wasMoved(z) -3.2 x = path z; body ``x = bitwiseCopy(path z);`` - do not emit `=destroy(x)`. Note: body - must not mutate ``z`` nor ``x``. All - assignments to ``x`` must be of the form - ``path z`` but the ``z`` can differ. - Neither ``z`` nor ``x`` can have the - flag ``sfAddrTaken`` to ensure no other - aliasing is going on. -4.1 y = sinkParam `=sink`(y, sinkParam) -4.2 x = y `=`(x, y) # a copy -5.1 f_sink(g()) f_sink(g()) -5.2 f_sink(y) f_sink(copy y); # copy unless we can see it's the last read -5.3 f_sink(move y) f_sink(y); wasMoved(y) # explicit moves empties 'y' -5.4 f_noSink(g()) var tmp = bitwiseCopy(g()); f(tmp); `=destroy`(tmp) - -Rule 3.2 describes a "cursor" variable, a variable that is only used as a -view into some data structure. See ``compiler/cursors.nim`` for details. - -Note: In order to avoid the very common combination ``reset(x); =sink(x, y)`` for -variable definitions we must turn "the first sink/assignment" operation into a -copyMem. This is harder than it looks: - - while true: - try: - if cond: break # problem if we run destroy(x) here :-/ - var x = f() - finally: - destroy(x) - -And the C++ optimizers don't sweat to optimize it for us, so we don't have -to do it. -]# import intsets, ast, astalgo, msgs, renderer, magicsys, types, idents, strutils, options, dfa, lowerings, tables, modulegraphs, msgs, lineinfos, parampatterns, sighashes -const - InterestingSyms = {skVar, skResult, skLet, skForVar, skTemp} - type Con = object owner: PSym @@ -217,43 +95,6 @@ proc isLastRead(n: PNode; c: var Con): bool = dbg: echo "ugh ", c.otherRead.isNil, " ", result - when false: - let s = n.sym - var pcs: seq[int] = @[instr+1] - var takenGotos: IntSet - var takenForks = initIntSet() - while pcs.len > 0: - var pc = pcs.pop - - takenGotos = initIntSet() - while pc < c.g.len: - case c.g[pc].kind - of def: - if c.g[pc].sym == s: - # the path lead to a redefinition of 's' --> abandon it. - break - inc pc - of use: - if c.g[pc].sym == s: - c.otherRead = c.g[pc].n - return false - inc pc - of goto: - # we must leave endless loops eventually: - if not takenGotos.containsOrIncl(pc): - pc = pc + c.g[pc].dest - else: - inc pc - of fork: - # we follow the next instruction but push the dest onto our "work" stack: - if not takenForks.containsOrIncl(pc): - pcs.add pc + c.g[pc].dest - inc pc - of InstrKind.join: - inc pc - #echo c.graph.config $ n.info, " last read here!" - return true - proc initialized(code: ControlFlowGraph; pc: int, init, uninit: var IntSet; comesFrom: int): int = ## Computes the set of definitely initialized variables across all code paths @@ -290,9 +131,6 @@ proc initialized(code: ControlFlowGraph; pc: int, inc pc return pc -template interestingSym(s: PSym): bool = - s.owner == c.owner and s.kind in InterestingSyms and hasDestructor(s.typ) - template isUnpackedTuple(s: PSym): bool = ## we move out all elements of unpacked tuples, ## hence unpacked tuples themselves don't need to be destroyed @@ -353,8 +191,8 @@ proc canBeMoved(t: PType): bool {.inline.} = let t = t.skipTypes({tyGenericInst, tyAlias, tySink}) result = t.kind != tyRef and t.attachedOps[attachedSink] != nil -proc genSink(c: Con; t: PType; dest, ri: PNode): PNode = - let t = t.skipTypes({tyGenericInst, tyAlias, tySink}) +proc genSink(c: Con; dest, ri: PNode): PNode = + let t = dest.typ.skipTypes({tyGenericInst, tyAlias, tySink}) let k = if t.attachedOps[attachedSink] != nil: attachedSink else: attachedAsgn if t.attachedOps[k] != nil: @@ -365,20 +203,20 @@ proc genSink(c: Con; t: PType; dest, ri: PNode): PNode = # we generate a fast assignment in this case: result = newTree(nkFastAsgn, dest) -proc genCopy(c: var Con; t: PType; dest, ri: PNode): PNode = +proc genCopyNoCheck(c: Con; dest, ri: PNode): PNode = + let t = dest.typ.skipTypes({tyGenericInst, tyAlias, tySink}) + result = genOp(c, t, attachedAsgn, dest, ri) + +proc genCopy(c: var Con; dest, ri: PNode): PNode = + let t = dest.typ if tfHasOwned in t.flags: # try to improve the error message here: if c.otherRead == nil: discard isLastRead(ri, c) checkForErrorPragma(c, t, ri, "=") - let t = t.skipTypes({tyGenericInst, tyAlias, tySink}) - result = genOp(c, t, attachedAsgn, dest, ri) + genCopyNoCheck(c, dest, ri) -proc genCopyNoCheck(c: Con; t: PType; dest, ri: PNode): PNode = - let t = t.skipTypes({tyGenericInst, tyAlias, tySink}) - result = genOp(c, t, attachedAsgn, dest, ri) - -proc genDestroy(c: Con; t: PType; dest: PNode): PNode = - let t = t.skipTypes({tyGenericInst, tyAlias, tySink}) +proc genDestroy(c: Con; dest: PNode): PNode = + let t = dest.typ.skipTypes({tyGenericInst, tyAlias, tySink}) result = genOp(c, t, attachedDestructor, dest, nil) proc addTopVar(c: var Con; v: PNode) = @@ -390,20 +228,10 @@ proc getTemp(c: var Con; typ: PType; info: TLineInfo): PNode = result = newSymNode(sym) c.addTopVar(result) -proc p(n: PNode; c: var Con): PNode - -template recurse(n, dest) = - for i in 0.. 0 and n.typ != nil and isDangerousSeq(n.typ): @@ -467,19 +299,66 @@ proc containsConstSeq(n: PNode): bool = of nkExprEqExpr, nkExprColonExpr, nkHiddenStdConv, nkHiddenSubConv: result = containsConstSeq(n[1]) of nkObjConstr, nkClosure: - for i in 1 ..< n.len: + for i in 1.. 0: + ri = genDefaultCall(v.typ, c, v.info) + if ri.kind != nkEmpty: + let r = moveOrCopy(v, ri, c) + result.add r + else: # keep the var but transform 'ri': + var v = copyNode(n) + var itCopy = copyNode(it) + for j in 0.. 0 and isDangerousSeq(ri.typ): - result = genCopy(c, dest.typ, dest, ri) + result = genCopy(c, dest, ri) else: - result = genSink(c, dest.typ, dest, ri) - let ri2 = copyTree(ri) - for i in 0.. 0: - ri = genDefaultCall(v.typ, c, v.info) - if ri.kind != nkEmpty: - let r = moveOrCopy(v, ri, c) - result.add r - else: - result.add keepVar(n, it, c) - of nkCallKinds: - let parameters = n[0].typ - let L = if parameters != nil: parameters.len else: 0 - for i in 1 ..< n.len: - n.sons[i] = pArg(n[i], c, i < L and isSinkTypeForParam(parameters[i])) - if n.typ != nil and hasDestructor(n.typ): - discard "produce temp creation" - result = newNodeIT(nkStmtListExpr, n.info, n.typ) - let tmp = getTemp(c, n.typ, n.info) - var sinkExpr = genSink(c, n.typ, tmp, n) - sinkExpr.add n - result.add sinkExpr - result.add tmp - c.destroys.add genDestroy(c, n.typ, tmp) - else: - result = n - of nkAsgn, nkFastAsgn: - if hasDestructor(n[0].typ) and n[1].kind notin {nkProcDef, nkDo, nkLambda}: - # rule (self-assignment-removal): - if n[1].kind == nkSym and n[0].kind == nkSym and n[0].sym == n[1].sym: - result = newNodeI(nkEmpty, n.info) - else: - result = moveOrCopy(n[0], n[1], c) - else: - result = copyNode(n) - recurse(n, result) - of nkNone..nkNilLit, nkTypeSection, nkProcDef, nkConverterDef, nkMethodDef, - nkIteratorDef, nkMacroDef, nkTemplateDef, nkLambda, nkDo, nkFuncDef: - result = n - of nkCast, nkHiddenStdConv, nkHiddenSubConv, nkConv: - result = copyNode(n) - # Destination type - result.add n[0] - # Analyse the inner expression - result.add p(n[1], c) - of nkWhen: - # This should be a "when nimvm" node. - result = copyTree(n) - result[1][0] = p(result[1][0], c) - of nkRaiseStmt: - if optNimV2 in c.graph.config.globalOptions and n[0].kind != nkEmpty: - if n[0].kind in nkCallKinds: - let call = copyNode(n[0]) - recurse(n[0], call) - result = copyNode(n) - result.add call - else: - let t = n[0].typ - let tmp = getTemp(c, t, n.info) - var m = genCopyNoCheck(c, t, tmp, n[0]) - - m.add p(n[0], c) - result = newTree(nkStmtList, genWasMoved(tmp, c), m) - var toDisarm = n[0] - if toDisarm.kind == nkStmtListExpr: toDisarm = toDisarm.lastSon - if toDisarm.kind == nkSym and toDisarm.sym.owner == c.owner: - result.add genWasMoved(toDisarm, c) - result.add newTree(nkRaiseStmt, tmp) - else: - result = copyNode(n) - recurse(n, result) - of nkForStmt, nkParForStmt, nkWhileStmt: - inc c.inLoop - result = copyNode(n) - recurse(n, result) - dec c.inLoop - else: - result = copyNode(n) - recurse(n, result) - proc extractDestroysForTemporaries(c: Con, destroys: PNode): PNode = result = newNodeI(nkStmtList, destroys.info) - for i in 0 ..< destroys.len: + for i in 0.. 0: result.add c.topLevelVars if c.destroys.len > 0: - reverseDestroys(c.destroys) + c.destroys.sons = reverseDestroys(c.destroys.sons) if owner.kind == skModule: result.add newTryFinally(body, extractDestroysForTemporaries(c, c.destroys)) g.globalDestructors.add c.destroys @@ -898,8 +675,6 @@ proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode = result.add newTryFinally(body, c.destroys) else: result.add body - dbg: - echo "------------------------------------" - echo owner.name.s, " transformed to: " + echo ">---------transformed-to--------->" echo result diff --git a/compiler/pragmas.nim b/compiler/pragmas.nim index f8f6a1a23b..208f6dae3d 100644 --- a/compiler/pragmas.nim +++ b/compiler/pragmas.nim @@ -66,7 +66,7 @@ const varPragmas* = declPragmas + {wVolatile, wRegister, wThreadVar, wMagic, wHeader, wCompilerProc, wCore, wDynlib, wNoInit, wCompileTime, wGlobal, - wGensym, wInject, wCodegenDecl, wGuard, wGoto, wCursor} + wGensym, wInject, wCodegenDecl, wGuard, wGoto} constPragmas* = declPragmas + {wHeader, wMagic, wGensym, wInject, wIntDefine, wStrDefine, wBoolDefine, wCompilerProc, wCore} @@ -1103,11 +1103,6 @@ proc singlePragma(c: PContext, sym: PSym, n: PNode, i: var int, invalidPragma(c, it) else: sym.flags.incl sfGoto - of wCursor: - if sym == nil or sym.kind notin {skVar, skLet}: - invalidPragma(c, it) - else: - sym.flags.incl sfCursor of wExportNims: if sym == nil: invalidPragma(c, it) else: magicsys.registerNimScriptSymbol(c.graph, sym) diff --git a/compiler/transf.nim b/compiler/transf.nim index 7521fe1694..afbe419507 100644 --- a/compiler/transf.nim +++ b/compiler/transf.nim @@ -663,7 +663,6 @@ proc transformFor(c: PTransf, n: PNode): PTransNode = t = arg.typ # generate a temporary and produce an assignment statement: var temp = newTemp(c, t, formal.info) - #temp.sym.flags.incl sfCursor addVar(v, temp) add(stmtList, newAsgnStmt(c, nkFastAsgn, temp, arg.PTransNode)) idNodeTablePut(newC.mapping, formal, temp) diff --git a/compiler/wordrecg.nim b/compiler/wordrecg.nim index 22715958f8..4650f22ecc 100644 --- a/compiler/wordrecg.nim +++ b/compiler/wordrecg.nim @@ -37,8 +37,6 @@ type wMagic, wThread, wFinal, wProfiler, wMemTracker, wObjChecks, wIntDefine, wStrDefine, wBoolDefine - wCursor, - wImmediate, wConstructor, wDestructor, wDelegator, wOverride, wImportCpp, wImportObjC, wImportCompilerProc, @@ -125,8 +123,6 @@ const "magic", "thread", "final", "profiler", "memtracker", "objchecks", "intdefine", "strdefine", "booldefine", - "cursor", - "immediate", "constructor", "destructor", "delegator", "override", "importcpp", "importobjc", "importcompilerproc", "importc", "importjs", "exportc", "exportcpp", "exportnims", diff --git a/doc/nimc.rst b/doc/nimc.rst index ebdd00063c..d53e3f310c 100644 --- a/doc/nimc.rst +++ b/doc/nimc.rst @@ -483,7 +483,6 @@ number information are given if the program crashes or an uncaught exception is raised. - DynlibOverride ============== @@ -497,14 +496,6 @@ on Linux:: nim c --dynlibOverride:lua --passL:liblua.lib program.nim -Cursor pragma -============= - -The ``.cursor`` pragma is a temporary tool for optimization purposes -and this property will be computed by Nim's optimizer eventually. Thus it -remains undocumented. - - Backend language options ========================