Skip to content

Commit

Permalink
cgen: remove dead code (nim-lang#1051)
Browse files Browse the repository at this point in the history
## Summary

Remove an optimization/semantic-change from `cgen` that's not used
anymore.

## Details

The `isInactiveDestructorCall`/`notYetAlive` procedures was responsible
for detecting whether a destructor call or reset operation could be
safely omitted for a local.

This is a workaround for destructor injection being limited to wrapping
whole scopes in `try`/`finally` statements, but it finally stopped
working with the introduction of the MIR (~11 months ago), as the 'def'
statements for all locals requiring destruction are placed before the
`try` statement responsible for their destruction.

Since the proper fix will be implemented in the destructor injection
pass rather than in the code generators, `isInactiveDestructorCall`,
`notYetAlive` are removed and their usage sites are updated. The now-
unused `compat.getRoot` procedure is removed too.
  • Loading branch information
zerbina committed Dec 9, 2023
1 parent 5d4430e commit 402e1e9
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 49 deletions.
26 changes: 0 additions & 26 deletions compiler/backend/ccgcalls.nim
Original file line number Diff line number Diff line change
Expand Up @@ -364,33 +364,7 @@ proc genClosureCall(p: BProc, le, ri: CgNode, d: var TLoc) =
genCallPattern()
exitCall(p, ri[0], canRaise)

proc notYetAlive(p: BProc, n: CgNode): bool {.inline.} =
let r = getRoot(n)
result = r != nil and r.kind == cnkLocal and p.locals[r.local].k == locNone

proc isInactiveDestructorCall(p: BProc, e: CgNode): bool =
#[ Consider this example.
var :tmpD_3281815
try:
if true:
return
let args_3280013 =
wasMoved_3281816(:tmpD_3281815)
`=_3280036`(:tmpD_3281815, [1])
:tmpD_3281815
finally:
`=destroy_3280027`(args_3280013)
We want to return early but the 'finally' section is traversed before
the 'let args = ...' statement. We exploit this to generate better
code for 'return'. ]#
result = e.len == 2 and e[0].kind == cnkSym and
e[0].sym.name.s == "=destroy" and notYetAlive(p, e[1].operand)

proc genAsgnCall(p: BProc, le, ri: CgNode, d: var TLoc) =
if p.withinBlockLeaveActions > 0 and isInactiveDestructorCall(p, ri):
return
if ri[0].typ.skipTypes({tyGenericInst, tyAlias, tySink}).callConv == ccClosure:
genClosureCall(p, le, ri, d)
else:
Expand Down
4 changes: 1 addition & 3 deletions compiler/backend/ccgexprs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1679,9 +1679,7 @@ proc skipAddr(n: CgNode): CgNode =
proc genWasMoved(p: BProc; n: CgNode) =
var a: TLoc
let n1 = n[1].skipAddr
if p.withinBlockLeaveActions > 0 and notYetAlive(p, n1):
discard
else:
if true:
initLocExpr(p, n1, a, {lfWantLvalue})
resetLoc(p, a)
#linefmt(p, cpsStmts, "#nimZeroMem((void*)$1, sizeof($2));$n",
Expand Down
20 changes: 0 additions & 20 deletions compiler/backend/compat.nim
Original file line number Diff line number Diff line change
Expand Up @@ -98,26 +98,6 @@ proc isDeepConstExpr*(n: CgNode): bool =
else:
result = false

proc getRoot*(n: CgNode): CgNode =
## ``getRoot`` takes a *path* ``n``. A path is an lvalue expression
## like ``obj.x[i].y``. The *root* of a path is the symbol that can be
## determined as the owner; ``obj`` in the example.
case n.kind
of cnkSym:
if n.sym.kind in {skVar, skLet, skForVar}:
result = n
of cnkLocal:
result = n
of cnkFieldAccess, cnkArrayAccess, cnkTupleAccess, cnkCheckedFieldAccess:
result = getRoot(n[0])
of cnkDerefView, cnkDeref, cnkObjUpConv, cnkObjDownConv, cnkHiddenAddr,
cnkAddr, cnkHiddenConv, cnkConv:
result = getRoot(n.operand)
of cnkCall:
if getMagic(n) == mSlice:
result = getRoot(n[1])
else: discard

proc canRaiseConservative*(fn: CgNode): bool =
## Duplicate of `canRaiseConservative <ast_query.html#canRaiseConservative,PNode>`_.
# ``mNone`` is also included in the set, therefore this check works even for
Expand Down

0 comments on commit 402e1e9

Please sign in to comment.