From aa7cf17f845caea9f7eafcff2aa0ece08aed3d9c Mon Sep 17 00:00:00 2001 From: Andreas Rumpf Date: Mon, 30 Nov 2020 23:36:38 +0100 Subject: [PATCH] Revert "fix #16185 (#16195)" (#16197) This reverts commit bb4b27a2ca414f06fbb9d14ff76fa02a088ac141. --- compiler/injectdestructors.nim | 119 ++++++++++++++++----------------- compiler/trees.nim | 15 ----- tests/arc/t14383.nim | 51 +------------- 3 files changed, 60 insertions(+), 125 deletions(-) diff --git a/compiler/injectdestructors.nim b/compiler/injectdestructors.nim index eba270eea80d8..34c11e06ca20b 100644 --- a/compiler/injectdestructors.nim +++ b/compiler/injectdestructors.nim @@ -19,7 +19,7 @@ import lineinfos, parampatterns, sighashes, liftdestructors, optimizer, varpartitions -from trees import exprStructuralEquivalent, getRoot, sameLocation +from trees import exprStructuralEquivalent, getRoot type Scope = object # well we do scope-based memory management. \ @@ -969,68 +969,67 @@ proc p(n: PNode; c: var Con; s: var Scope; mode: ProcessMode): PNode = internalError(c.graph.config, n.info, "cannot inject destructors to node kind: " & $n.kind) proc moveOrCopy(dest, ri: PNode; c: var Con; s: var Scope, isDecl = false): PNode = - if sameLocation(dest, ri): - # rule (self-assignment-removal): - result = newNodeI(nkEmpty, dest.info) - else: - case ri.kind - of nkCallKinds: + case ri.kind + of nkCallKinds: + result = c.genSink(dest, p(ri, c, s, consumed), isDecl) + of nkBracketExpr: + if isUnpackedTuple(ri[0]): + # unpacking of tuple: take over the elements result = c.genSink(dest, p(ri, c, s, consumed), isDecl) - of nkBracketExpr: - 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)) - else: - result = c.genCopy(dest, ri) - result.add p(ri, c, s, consumed) - c.finishCopy(result, dest, isFromSink = false) - of nkBracket: - # array constructor - if ri.len > 0 and isDangerousSeq(ri.typ): - result = c.genCopy(dest, ri) - result.add p(ri, c, s, consumed) - c.finishCopy(result, dest, isFromSink = false) - else: - result = c.genSink(dest, p(ri, c, s, consumed), isDecl) - of nkObjConstr, nkTupleConstr, nkClosure, nkCharLit..nkNilLit: + 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)) + else: + result = c.genCopy(dest, ri) + result.add p(ri, c, s, consumed) + c.finishCopy(result, dest, isFromSink = false) + of nkBracket: + # array constructor + if ri.len > 0 and isDangerousSeq(ri.typ): + result = c.genCopy(dest, ri) + result.add p(ri, c, s, consumed) + c.finishCopy(result, dest, isFromSink = false) + else: result = c.genSink(dest, p(ri, c, s, consumed), isDecl) - of nkSym: - if isSinkParam(ri.sym) and isLastRead(ri, c): - # Rule 3: `=sink`(x, z); wasMoved(z) - let snk = c.genSink(dest, ri, isDecl) - result = newTree(nkStmtList, snk, c.genWasMoved(ri)) - elif ri.sym.kind != skParam and ri.sym.owner == c.owner and - isLastRead(ri, c) and canBeMoved(c, dest.typ) and not isCursor(ri, c): - # Rule 3: `=sink`(x, z); wasMoved(z) - let snk = c.genSink(dest, ri, isDecl) - result = newTree(nkStmtList, snk, c.genWasMoved(ri)) - else: - result = c.genCopy(dest, ri) - result.add p(ri, c, s, consumed) - c.finishCopy(result, dest, isFromSink = false) - of nkHiddenSubConv, nkHiddenStdConv, nkConv, nkObjDownConv, nkObjUpConv: - result = c.genSink(dest, p(ri, c, s, sinkArg), isDecl) - of nkStmtListExpr, nkBlockExpr, nkIfExpr, nkCaseStmt, nkTryStmt: - template process(child, s): untyped = moveOrCopy(dest, child, c, s, isDecl) - # We know the result will be a stmt so we use that fact to optimize - handleNestedTempl(ri, process, willProduceStmt = true) - of nkRaiseStmt: - result = pRaiseStmt(ri, c, s) + of nkObjConstr, nkTupleConstr, nkClosure, nkCharLit..nkNilLit: + result = c.genSink(dest, p(ri, c, s, consumed), isDecl) + of nkSym: + if dest.kind == nkSym and dest.sym == ri.sym: + # rule (self-assignment-removal): + result = newNodeI(nkEmpty, dest.info) + elif isSinkParam(ri.sym) and isLastRead(ri, c): + # Rule 3: `=sink`(x, z); wasMoved(z) + let snk = c.genSink(dest, ri, isDecl) + result = newTree(nkStmtList, snk, c.genWasMoved(ri)) + elif ri.sym.kind != skParam and ri.sym.owner == c.owner and + isLastRead(ri, c) and canBeMoved(c, dest.typ) and not isCursor(ri, c): + # Rule 3: `=sink`(x, z); wasMoved(z) + let snk = c.genSink(dest, ri, isDecl) + result = newTree(nkStmtList, snk, c.genWasMoved(ri)) else: - if isAnalysableFieldAccess(ri, c.owner) and isLastRead(ri, c) and - canBeMoved(c, dest.typ): - # Rule 3: `=sink`(x, z); wasMoved(z) - let snk = c.genSink(dest, ri, isDecl) - result = newTree(nkStmtList, snk, c.genWasMoved(ri)) - else: - result = c.genCopy(dest, ri) - result.add p(ri, c, s, consumed) - c.finishCopy(result, dest, isFromSink = false) + result = c.genCopy(dest, ri) + result.add p(ri, c, s, consumed) + c.finishCopy(result, dest, isFromSink = false) + of nkHiddenSubConv, nkHiddenStdConv, nkConv, nkObjDownConv, nkObjUpConv: + result = c.genSink(dest, p(ri, c, s, sinkArg), isDecl) + of nkStmtListExpr, nkBlockExpr, nkIfExpr, nkCaseStmt, nkTryStmt: + template process(child, s): untyped = moveOrCopy(dest, child, c, s, isDecl) + # We know the result will be a stmt so we use that fact to optimize + handleNestedTempl(ri, process, willProduceStmt = true) + of nkRaiseStmt: + result = pRaiseStmt(ri, c, s) + else: + if isAnalysableFieldAccess(ri, c.owner) and isLastRead(ri, c) and + canBeMoved(c, dest.typ): + # Rule 3: `=sink`(x, z); wasMoved(z) + let snk = c.genSink(dest, ri, isDecl) + result = newTree(nkStmtList, snk, c.genWasMoved(ri)) + else: + result = c.genCopy(dest, ri) + result.add p(ri, c, s, consumed) + c.finishCopy(result, dest, isFromSink = false) proc computeUninit(c: var Con) = if not c.uninitComputed: diff --git a/compiler/trees.nim b/compiler/trees.nim index eb88f8d671d9a..05c0605953736 100644 --- a/compiler/trees.nim +++ b/compiler/trees.nim @@ -54,21 +54,6 @@ proc exprStructuralEquivalent*(a, b: PNode; strictSymEquality=false): bool = strictSymEquality): return result = true -proc sameLocation*(a, b: PNode): bool = - if a == b: true - elif a == nil or b == nil: false - elif a.kind == b.kind: - case a.kind: - of nkCheckedFieldExpr, nkDerefExpr, nkHiddenDeref, - nkAddr, nkHiddenAddr, nkObjDownConv, nkObjUpConv: - sameLocation(a[0], b[0]) - of nkDotExpr, nkBracketExpr: - sameLocation(a[0], b[0]) and sameLocation(a[1], b[1]) - of nkHiddenStdConv, nkHiddenSubConv: sameLocation(a[1], b[1]) - of nkNone..nkNilLit: exprStructuralEquivalent(a, b, strictSymEquality = true) - else: false - else: false - proc sameTree*(a, b: PNode): bool = if a == b: result = true diff --git a/tests/arc/t14383.nim b/tests/arc/t14383.nim index 0ce7b7d10fcfe..85f90d1c8ec83 100644 --- a/tests/arc/t14383.nim +++ b/tests/arc/t14383.nim @@ -125,53 +125,4 @@ proc main = let rankdef = avals echo avals.len, " ", rankdef.len -main() - - - - -#------------------------------------------------------------------------------ -# Issue #16185, complex self-assingment elimination -#------------------------------------------------------------------------------ - -type - CpuStorage*[T] = ref CpuStorageObj[T] - CpuStorageObj[T] = object - size*: int - raw_buffer*: ptr UncheckedArray[T] - Tensor[T] = object - buf*: CpuStorage[T] - TestObject = object - x: Tensor[float] - -proc `=destroy`[T](s: var CpuStorageObj[T]) = - if s.raw_buffer != nil: - s.raw_buffer.deallocShared() - s.size = 0 - s.raw_buffer = nil - -proc `=`[T](a: var CpuStorageObj[T]; b: CpuStorageObj[T]) {.error.} - -proc allocCpuStorage[T](s: var CpuStorage[T], size: int) = - new(s) - s.raw_buffer = cast[ptr UncheckedArray[T]](allocShared0(sizeof(T) * size)) - s.size = size - -proc newTensor[T](size: int): Tensor[T] = - allocCpuStorage(result.buf, size) - -proc `[]`[T](t: Tensor[T], idx: int): T = t.buf.raw_buffer[idx] -proc `[]=`[T](t: Tensor[T], idx: int, val: T) = t.buf.raw_buffer[idx] = val - -proc toTensor[T](s: seq[T]): Tensor[T] = - result = newTensor[T](s.len) - for i, x in s: - result[i] = x - -proc main2() = - var t: TestObject - t.x = toTensor(@[1.0, 2, 3, 4]) - t.x = t.x - doAssert(t.x.buf != nil) # self-assignment above should be eliminated - -main2() +main() \ No newline at end of file