mirror of
https://github.com/nim-lang/Nim.git
synced 2026-01-09 14:32:53 +00:00
* fix #16185 * fix test * fix comment * fix comment * better approach * Add more tests and move sameLocation to injectdestructors * Better and more strict sameLocation * Small cleanup and preliminary spec clarification * Fix * Fix doc * Expand test Co-authored-by: Andrey R (cooldome) <ariabushenko@gmail.com>
This commit is contained in:
Clyybber
@@ -968,68 +968,95 @@ proc p(n: PNode; c: var Con; s: var Scope; mode: ProcessMode): PNode =
|
||||
else:
|
||||
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 =
|
||||
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)
|
||||
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:
|
||||
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:
|
||||
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)
|
||||
proc sameLocation*(a, b: PNode): bool =
|
||||
proc sameConstant(a, b: PNode): bool =
|
||||
a.kind in nkLiterals and exprStructuralEquivalent(a, b)
|
||||
|
||||
const nkEndPoint = {nkSym, nkDotExpr, nkCheckedFieldExpr, nkBracketExpr}
|
||||
if a.kind in nkEndPoint and b.kind in nkEndPoint:
|
||||
if a.kind == b.kind:
|
||||
case a.kind
|
||||
of nkSym: a.sym == b.sym
|
||||
of nkDotExpr, nkCheckedFieldExpr: sameLocation(a[0], b[0]) and sameLocation(a[1], b[1])
|
||||
of nkBracketExpr: sameLocation(a[0], b[0]) and sameConstant(a[1], b[1])
|
||||
else: false
|
||||
else: false
|
||||
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))
|
||||
case a.kind
|
||||
of nkSym, nkDotExpr, nkCheckedFieldExpr, nkBracketExpr:
|
||||
# Reached an endpoint, flip to recurse the other side.
|
||||
sameLocation(b, a)
|
||||
of nkAddr, nkHiddenAddr, nkDerefExpr, nkHiddenDeref:
|
||||
# We don't need to check addr/deref levels or differentiate between the two,
|
||||
# since pointers don't have hooks :) (e.g: var p: ptr pointer; p[] = addr p)
|
||||
sameLocation(a[0], b)
|
||||
of nkObjDownConv, nkObjUpConv: sameLocation(a[0], b)
|
||||
of nkHiddenStdConv, nkHiddenSubConv: sameLocation(a[1], b)
|
||||
else: false
|
||||
|
||||
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:
|
||||
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:
|
||||
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)
|
||||
else:
|
||||
result = c.genCopy(dest, ri)
|
||||
result.add p(ri, c, s, consumed)
|
||||
c.finishCopy(result, dest, isFromSink = false)
|
||||
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:
|
||||
|
||||
@@ -376,8 +376,14 @@ it's subtle.
|
||||
|
||||
The simple case of ``x = x`` cannot be turned
|
||||
into ``=sink(x, x); wasMoved(x)`` because that would lose ``x``'s value.
|
||||
The solution is that simple self-assignments are simply transformed into
|
||||
an empty statement that does nothing.
|
||||
The solution is that simple self-assignments that consist of
|
||||
|
||||
- Symbols: ``x = x``
|
||||
- Field access: ``x.f = x.f``
|
||||
- Array, sequence or string access with indices known at compile-time: ``x[0] = x[0]``
|
||||
|
||||
are transformed into an empty statement that does nothing.
|
||||
The compiler is free to optimize further cases.
|
||||
|
||||
The complex case looks like a variant of ``x = f(x)``, we consider
|
||||
``x = select(rand() < 0.5, x, y)`` here:
|
||||
|
||||
@@ -168,3 +168,50 @@ proc testRefs() =
|
||||
testRefs()
|
||||
|
||||
doAssert(dDestroyed == 2)
|
||||
|
||||
|
||||
#------------------------------------------------------------------------------
|
||||
# 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()
|
||||
|
||||
@@ -73,6 +73,25 @@ bye
|
||||
()
|
||||
()
|
||||
()
|
||||
1
|
||||
destroy
|
||||
1
|
||||
destroy
|
||||
1
|
||||
destroy
|
||||
copy (self-assign)
|
||||
1
|
||||
destroy
|
||||
1
|
||||
destroy
|
||||
1
|
||||
destroy
|
||||
destroy
|
||||
copy
|
||||
@[(f: 2), (f: 2), (f: 3)]
|
||||
destroy
|
||||
destroy
|
||||
destroy
|
||||
'''
|
||||
"""
|
||||
|
||||
@@ -556,4 +575,96 @@ let w2 = newWrapper2(1)
|
||||
echo $w2[]
|
||||
|
||||
let w3 = newWrapper2(-1)
|
||||
echo $w3[]
|
||||
echo $w3[]
|
||||
|
||||
|
||||
#---------------------------------------------------------------------
|
||||
#self-assignments
|
||||
|
||||
# Self-assignments that are not statically determinable will get
|
||||
# turned into `=copy` calls as caseBracketExprCopy demonstrates.
|
||||
# (`=copy` handles self-assignments at runtime)
|
||||
|
||||
type
|
||||
OO = object
|
||||
f: int
|
||||
W = object
|
||||
o: OO
|
||||
|
||||
proc `=destroy`(x: var OO) =
|
||||
if x.f != 0:
|
||||
echo "destroy"
|
||||
x.f = 0
|
||||
|
||||
proc `=sink`(x: var OO, y: OO) =
|
||||
`=destroy`(x)
|
||||
echo "sink"
|
||||
x.f = y.f
|
||||
|
||||
proc `=copy`(x: var OO, y: OO) =
|
||||
if x.f != y.f:
|
||||
`=destroy`(x)
|
||||
echo "copy"
|
||||
x.f = y.f
|
||||
else:
|
||||
echo "copy (self-assign)"
|
||||
|
||||
proc caseSym =
|
||||
var o = OO(f: 1)
|
||||
o = o # NOOP
|
||||
echo o.f # "1"
|
||||
# "destroy"
|
||||
|
||||
caseSym()
|
||||
|
||||
proc caseDotExpr =
|
||||
var w = W(o: OO(f: 1))
|
||||
w.o = w.o # NOOP
|
||||
echo w.o.f # "1"
|
||||
# "destroy"
|
||||
|
||||
caseDotExpr()
|
||||
|
||||
proc caseBracketExpr =
|
||||
var w = [0: OO(f: 1)]
|
||||
w[0] = w[0] # NOOP
|
||||
echo w[0].f # "1"
|
||||
# "destroy"
|
||||
|
||||
caseBracketExpr()
|
||||
|
||||
proc caseBracketExprCopy =
|
||||
var w = [0: OO(f: 1)]
|
||||
let i = 0
|
||||
w[i] = w[0] # "copy (self-assign)"
|
||||
echo w[0].f # "1"
|
||||
# "destroy"
|
||||
|
||||
caseBracketExprCopy()
|
||||
|
||||
proc caseDotExprAddr =
|
||||
var w = W(o: OO(f: 1))
|
||||
w.o = addr(w.o)[] # NOOP
|
||||
echo w.o.f # "1"
|
||||
# "destroy"
|
||||
|
||||
caseDotExprAddr()
|
||||
|
||||
proc caseBracketExprAddr =
|
||||
var w = [0: OO(f: 1)]
|
||||
addr(w[0])[] = addr(addr(w[0])[])[] # NOOP
|
||||
echo w[0].f # "1"
|
||||
# "destroy"
|
||||
|
||||
caseBracketExprAddr()
|
||||
|
||||
proc caseNotAConstant =
|
||||
var i = 0
|
||||
proc rand: int =
|
||||
result = i
|
||||
inc i
|
||||
var s = @[OO(f: 1), OO(f: 2), OO(f: 3)]
|
||||
s[rand()] = s[rand()] # "destroy" "copy"
|
||||
echo s # @[(f: 2), (f: 2), (f: 3)]
|
||||
|
||||
caseNotAConstant()
|
||||
|
||||
Reference in New Issue
Block a user