From af27e6bdea63bbf66718193ec44bc61e745ded38 Mon Sep 17 00:00:00 2001 From: Clyybber Date: Sat, 4 Jul 2020 17:45:07 +0200 Subject: Fix #14396 (#14793) * Correct Left-To-Right evaluation of proc args * Fix CPP backend * Add testcase * closes #14396 * closes #14345 * Improve test and optimize * Improve testcase and optimize literals * Fix bug * Expand testcase and use DFA to optimize * Turn genParams into proc * Turn withTmpIfNeeded into a proc * Cleanup * Fix crash * Better analysis * Cleanup * Trailing newline.. * Fix build * Tiny cleanup Co-authored-by: Andreas Rumpf --- compiler/ccgcalls.nim | 145 ++++++++++++++++++++++++++++++++-------------- compiler/closureiters.nim | 2 +- 2 files changed, 102 insertions(+), 45 deletions(-) (limited to 'compiler') diff --git a/compiler/ccgcalls.nim b/compiler/ccgcalls.nim index 2c5d306a3..5f99f357d 100644 --- a/compiler/ccgcalls.nim +++ b/compiler/ccgcalls.nim @@ -217,21 +217,30 @@ proc openArrayLoc(p: BProc, formalType: PType, n: PNode): Rope = internalError(p.config, "openArrayLoc: " & typeToString(a.t)) else: internalError(p.config, "openArrayLoc: " & typeToString(a.t)) -proc genArgStringToCString(p: BProc, n: PNode): Rope {.inline.} = +proc withTmpIfNeeded(p: BProc, a: TLoc, needsTmp: bool): TLoc = + if needsTmp: + var tmp: TLoc + getTemp(p, a.lode.typ, tmp, needsInit=false) + genAssignment(p, tmp, a, {}) + tmp + else: + a + +proc genArgStringToCString(p: BProc, n: PNode, needsTmp: bool): Rope {.inline.} = var a: TLoc initLocExpr(p, n[0], a) - result = ropecg(p.module, "#nimToCStringConv($1)", [a.rdLoc]) + ropecg(p.module, "#nimToCStringConv($1)", [withTmpIfNeeded(p, a, needsTmp).rdLoc]) -proc genArg(p: BProc, n: PNode, param: PSym; call: PNode): Rope = +proc genArg(p: BProc, n: PNode, param: PSym; call: PNode, needsTmp = false): Rope = var a: TLoc if n.kind == nkStringToCString: - result = genArgStringToCString(p, n) + result = genArgStringToCString(p, n, needsTmp) elif skipTypes(param.typ, abstractVar).kind in {tyOpenArray, tyVarargs}: var n = if n.kind != nkHiddenAddr: n else: n[0] result = openArrayLoc(p, param.typ, n) elif ccgIntroducedPtr(p.config, param, call[0].typ[0]): initLocExpr(p, n, a) - result = addrLoc(p.config, a) + result = addrLoc(p.config, withTmpIfNeeded(p, a, needsTmp)) elif p.module.compileToCpp and param.typ.kind in {tyVar} and n.kind == nkHiddenAddr: initLocExprSingleUse(p, n[0], a) @@ -246,26 +255,85 @@ proc genArg(p: BProc, n: PNode, param: PSym; call: PNode): Rope = result = rdLoc(a) else: initLocExprSingleUse(p, n, a) - result = rdLoc(a) + result = rdLoc(withTmpIfNeeded(p, a, needsTmp)) -proc genArgNoParam(p: BProc, n: PNode): Rope = +proc genArgNoParam(p: BProc, n: PNode, needsTmp = false): Rope = var a: TLoc if n.kind == nkStringToCString: - result = genArgStringToCString(p, n) + result = genArgStringToCString(p, n, needsTmp) else: initLocExprSingleUse(p, n, a) - result = rdLoc(a) + result = rdLoc(withTmpIfNeeded(p, a, needsTmp)) + +from dfa import instrTargets, InstrTargetKind + +proc potentialAlias(n: PNode, potentialWrites: seq[PNode]): bool = + for p in potentialWrites: + if instrTargets(p, n) != None: + return true + +proc skipTrivialIndirections(n: PNode): PNode = + result = n + while true: + case result.kind + of {nkDerefExpr, nkHiddenDeref, nkAddr, nkHiddenAddr, nkObjDownConv, nkObjUpConv}: + result = result[0] + of {nkHiddenStdConv, nkHiddenSubConv}: + result = result[1] + else: break + +proc getPotentialWrites(n: PNode, mutate = false): seq[PNode] = + case n.kind: + of nkLiterals, nkIdent: discard + of nkSym: + if mutate: result.add n + of nkAsgn, nkFastAsgn: + result.add getPotentialWrites(n[0], true) + result.add getPotentialWrites(n[1], mutate) + of nkAddr, nkHiddenAddr: + result.add getPotentialWrites(n[0], true) + of nkCallKinds: #TODO: Find out why in f += 1, f is a nkSym and not a nkHiddenAddr + for s in n.sons: + result.add getPotentialWrites(s, true) + else: + for s in n.sons: + result.add getPotentialWrites(s, mutate) -template genParamLoop(params) {.dirty.} = - if i < typ.len: - assert(typ.n[i].kind == nkSym) - let paramType = typ.n[i] - if not paramType.typ.isCompileTimeOnly: - if params != nil: params.add(~", ") - params.add(genArg(p, ri[i], paramType.sym, ri)) +proc getPotentialReads(n: PNode): seq[PNode] = + case n.kind: + of nkLiterals, nkIdent: discard + of nkSym: result.add n else: - if params != nil: params.add(~", ") - params.add(genArgNoParam(p, ri[i])) + for s in n.sons: + result.add getPotentialReads(s) + +proc genParams(p: BProc, ri: PNode, typ: PType): Rope = + # We must generate temporaries in cases like #14396 + # to keep the strict Left-To-Right evaluation + var needTmp = newSeq[bool](ri.len - 1) + var potentialWrites: seq[PNode] + for i in countdown(ri.len - 1, 1): + if ri[i].skipTrivialIndirections.kind == nkSym: + needTmp[i - 1] = potentialAlias(ri[i], potentialWrites) + else: + for n in getPotentialReads(ri[i]): + if not needTmp[i - 1]: + needTmp[i - 1] = potentialAlias(n, potentialWrites) + potentialWrites.add getPotentialWrites(ri[i]) + if ri[i].kind == nkHiddenAddr: + # Optimization: don't use a temp, if we would only take the adress anyway + needTmp[i - 1] = false + + for i in 1.. 0 and isInactiveDestructorCall(p, e): - return - if e[0].typ.skipTypes({tyGenericInst, tyAlias, tySink, tyOwned}).callConv == ccClosure: - genClosureCall(p, nil, e, d) - elif e[0].kind == nkSym and sfInfixCall in e[0].sym.flags: - genInfixCall(p, nil, e, d) - elif e[0].kind == nkSym and sfNamedParamCall in e[0].sym.flags: - genNamedParamCall(p, e, d) - else: - genPrefixCall(p, nil, e, d) - postStmtActions(p) - proc genAsgnCall(p: BProc, le, ri: PNode, d: var TLoc) = + if p.withinBlockLeaveActions > 0 and isInactiveDestructorCall(p, ri): + return if ri[0].typ.skipTypes({tyGenericInst, tyAlias, tySink, tyOwned}).callConv == ccClosure: genClosureCall(p, le, ri, d) elif ri[0].kind == nkSym and sfInfixCall in ri[0].sym.flags: @@ -691,3 +746,5 @@ proc genAsgnCall(p: BProc, le, ri: PNode, d: var TLoc) = else: genPrefixCall(p, le, ri, d) postStmtActions(p) + +proc genCall(p: BProc, e: PNode, d: var TLoc) = genAsgnCall(p, nil, e, d) diff --git a/compiler/closureiters.nim b/compiler/closureiters.nim index 675f1ae41..2248f956e 100644 --- a/compiler/closureiters.nim +++ b/compiler/closureiters.nim @@ -129,7 +129,7 @@ # break :stateLoop import - ast, astalgo, msgs, idents, + ast, msgs, idents, renderer, magicsys, lowerings, lambdalifting, modulegraphs, lineinfos type -- cgit 1.4.1-2-gfad0