Skip to content

Commit

Permalink
compiler: rewrite injectdestructors as MIR passes
Browse files Browse the repository at this point in the history
injectdestructors: fix steps being in the wrong order

analyser: fix and improve `computeSpan` and `sibling`

mirtrees: move `MirTree`-related parts to a separate module

analyser: `injectHooks` progress

analyser: correctly compute the nodes a scope spans

The start and end node of the `mnkBranch`/`mnkBlock` sub-trees must not
be included.

analyser: fix issue in `prepass`

analyser: run `prepass` first

analyser: fix `exprRoot`

- the traversal was implemented incorrectly
- calls yielding views weren't considered

analyser: progress on `isRootOf`

- properly walk the expression nodes
- compare symbol-storing atoms
- fix and clean up `mnlCall` comparison
- handle `mnkDeref`

mirtraverse: implement abstract execution

analyser: `canBorrow` progress

- use `sliceIt`
- general cleanup
- fix initialization of locations other than the analysed one being
  considered
- fix the `mnkArg` node being passed to `isLValue` and `handleUse`

analyser: use the new `traverse` where applicable

mirtraverse: use an instruction-based CFG

This is simpler and faster than processing the `MirNode`s on-the-fly.

tests: add the basics for testing `mirtraverse`

Actual test cases are not yet present.

tests: add two test cases

mirtraverse: implement backwards execution of the CFG

Make use the of the new algorithm in `injectHooks`.

mirtraverse: fix abstract-evaluation-order issue

The arguments to a call that can raise have to evaluated before the
'fork'.

mirtraverse: don't use 'fork' for a catch-all branch

analyser: consider complete assignments during ownership analysis

In the following example, this enables the assignments to both `y`
and `z` to be turned into moves:
```nim
y = x
x = ...
z = x
```

analyser: a `while` also opens a scope

analyser: use backwards analysis for `canBorrow`

analyser: allow `unaryOperand` for `mnkDeref`

analyser: make the `Changeset` API work

analyser: progress on `=sink` injection

analyser: only consider `sink` parameters with destructors

analyser: register `useSink` actions

move the `Changeset` implementation to a separate module

In addition, add documentation for some of the procedures.

split up application of 'borrow' and non-'borrow' actions

This is a preparation for the upcoming refactoring of the analyser
pipeline.

general progress

- split up alive- and ownership-analysis into two procedures
- split borrow-inference into a fully separate pass
- the analysis passes no longer produce actions. Instead, they now
  produce the data to infer the actions from
  - the `apply` procedures use said data to create changes directly
  - `prepass` is now unnecessary
- make basic destructor injection work
- make `=copy` and `=sink` hook injection work
- respect `.cursor` ins some parts (not all yet)
- improve documentation

analyser: fix the wrong node being analysed

The `mnkArg` node was passed to `isAnalysedLoc`, not it's operand.

analyser: conside constructor expression inputs...

...during ownership analysis.

mirtrees: remove the `fieldPos` field

This lowers the size of a `MirNode` by 8-byte. Tuple access now uses
`mnkPathArr`.

mirgen: translate `skTemp` symbols to MIR temporaries

Having two different types of temporaries is not a good idea and
complicated things.

[split off] sighashes: use `func`

Due to the forward declaration, `hashType` was treated as having
side-effects, preventing it from being used inside a `func`.

injectdestructors: don't process inline iterators and...

... generated ops.

analyser: remove non-working cursor/borrow inference

Re-implementing cursor inference is out-of-scope for this PR. The
existing implementation can be reused for now.

general cleanup

WIP: partial redesign of the MIR

analyser: fix issues with `hasInput`

- operations with no input were detected as having input when appearing
  as the first child-node of input sub-trees
- `void` lead to ambiguities. It still does, but in the case of
  `hasInput`, it's now ignored

analyser: split destructor injection into a separate procedure

analyser: implement proper destructor injection

Locations of which the value escapes are now correctly destroyed inside
a `finally` clause.

It's likely that the current implementation is not final.

injectdestructors: implement `mnkSwitch` lowering

injectdestructors: add self-assignment elimination

magicsys: add `getMagicEqForType`

The procedure is useful when one is only interested in the magic itself
and doesn't require a `PSym`.

analyser: add the `operands` procedure

In addition, make the `operand` procedure exported. Both are not
directly related to the analyser logic and are thus candidates for being
moved elsewhere.

analyser: consider sub-trees in `operand` and `operands`

analyser: add `findParent`

analyser: move general query procedures over to `mirtrees.nim`

mirgen: work around non-owning temporaries getting destroyed

analyser: add missing `mnkModify` node

`wasMoved` works without it, but omitting it is still a semantic error.

injectdestructors: ensure that constructed `ref` are consumed

mirtraverse: improve `computeCfg`

- clean up the procedure so that it relies less on templates
- simplify the insertion of `join` instructions
  - `maybeJoin` is no longer used
  - the instructions can now be sorted by their corresponding node
    position

post-merge fix-up

finish the rewrite

tests: add `ttest.nim`

adjust imports of `mirtraverse` (now `mirexec`)
  • Loading branch information
zerbina committed Feb 10, 2023
1 parent f175f5f commit 6849732
Show file tree
Hide file tree
Showing 10 changed files with 3,335 additions and 1,069 deletions.
616 changes: 616 additions & 0 deletions compiler/mir/analysis.nim

Large diffs are not rendered by default.

33 changes: 16 additions & 17 deletions compiler/modules/magicsys.nim
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import
compiler/utils/[
idioms,
platform,
],
compiler/ast/[
Expand Down Expand Up @@ -175,24 +176,22 @@ proc getNimScriptSymbol*(g: ModuleGraph; name: string): PSym =

proc resetNimScriptSymbols*(g: ModuleGraph) = initStrTable(g.exposed)

proc getMagicEqSymForType*(g: ModuleGraph; t: PType; info: TLineInfo): PSym =
func getMagicEqForType*(t: PType): TMagic =
## Returns the ``mEqX`` magic for the given type `t`.
case t.kind
of tyInt, tyInt8, tyInt16, tyInt32, tyInt64,
tyUInt, tyUInt8, tyUInt16, tyUInt32, tyUInt64:
result = getSysMagic(g, info, "==", mEqI)
of tyEnum:
result = getSysMagic(g, info, "==", mEqEnum)
of tyBool:
result = getSysMagic(g, info, "==", mEqB)
of tyRef, tyPtr, tyPointer:
result = getSysMagic(g, info, "==", mEqRef)
of tyString:
result = getSysMagic(g, info, "==", mEqStr)
of tyChar:
result = getSysMagic(g, info, "==", mEqCh)
of tySet:
result = getSysMagic(g, info, "==", mEqSet)
of tyProc:
result = getSysMagic(g, info, "==", mEqProc)
mEqI
of tyEnum: mEqEnum
of tyBool: mEqB
of tyRef, tyPtr, tyPointer: mEqRef
of tyString: mEqStr
of tyChar: mEqCh
of tySet: mEqSet
of tyProc: mEqProc
else:
g.config.globalReport(info, reportTyp(rsemNoMagicEqualsForType, t))
unreachable(t.kind)

proc getMagicEqSymForType*(g: ModuleGraph; t: PType; info: TLineInfo): PSym =
let magic = getMagicEqForType(t)
result = getSysMagic(g, info, "==", magic)
168 changes: 168 additions & 0 deletions compiler/sem/aliasanalysis.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
## This module implements a simple alias analysis based on MIR operation
## sequences, used to compute the relationship between two lvalues, i.e. if
## the underlying locations potentially overlap or are part sub- or
## super-locations of each other.
##
## Only lvalues with statically know roots (derived from named locals, global,
## etc.) are supported.
##
## The main procedure is ``compareLvalues``: it performs the comparison and
## returns the result as an opaque object ``CmpLocsResult``, which can then be
## queried for the relationship between the two inputs.
##
## Whether two lvalues refer to overlapping locations is not always statically
## known (e.g. when accessing an array with a value only known at run-time) --
## if it is not, the uncertainity is communicated via the ``maybe`` result.

import
compiler/ast/[
ast
],
compiler/mir/[
mirtrees
],
compiler/utils/[
idioms
]

type
Ternary* = enum
no, maybe, yes

LvalueExpr* = tuple[root, last: NodePosition]
# XXX: ``LvalueExpr`` fit well at one point, but it doesn't anymore. The
# name ``Handle`` might be a better fit now

CmpLocsResult* = object
endA, endB: bool ## whether the `last` operation of the provided sequences
## was reached
overlaps: Ternary

const
Roots = SymbolLike + {mnkTemp, mnkCall, mnkMagic, mnkDeref, mnkDerefView}

func isSameRoot(an, bn: MirNode): bool =
if an.kind != bn.kind:
return false

case an.kind
of SymbolLike:
result = an.sym.id == bn.sym.id
of mnkTemp:
result = an.temp == bn.temp
of Roots - SymbolLike - {mnkTemp}:
result = false
else:
unreachable(an.kind)

func sameIndex(a, b: MirNode): Ternary =
if a.kind != b.kind or a.kind != mnkLiteral:
maybe
else:
if a.lit.intVal == b.lit.intVal:
yes
else:
no

proc compareLvalues(body: MirTree, a, b: NodePosition,
lastA, lastB: NodePosition): CmpLocsResult =
## Performs the comparision and computes the information to later derive the
## facts from (e.g. if A is a part B)
if not isSameRoot(body[a], body[b]):
return CmpLocsResult(overlaps: no)

var
a = a + 1
b = b + 1

const IgnoreSet = {mnkStdConv, mnkConv, mnkAddr, mnkView}
## we can skip all conversions, since we know that they must be lvalue
## conversions; ``compareLvalues`` is only used for lvalues, which
## can't result from non-lvalue conversions.
## Both the 'addr' and 'view' operatio create a first-class handle to
## the reference location, so we also skip them. This allows for ``a.x``
## to be treated as refering to same location as ``a.x.addr`` (which is
## correct)

var overlaps = yes # until proven otherwise
while overlaps != no and a <= lastA and b <= lastB:
while body[a].kind in IgnoreSet:
inc a

while body[b].kind in IgnoreSet:
inc b

if a > lastA or b > lastB:
break

let
an {.cursor.} = body[a]
bn {.cursor.} = body[b]

if an.kind != bn.kind:
overlaps = no
break

case an.kind
of mnkPathNamed, mnkPathVariant:
if an.field.id != bn.field.id:
overlaps = no
break

of mnkPathPos:
if an.position != bn.position:
overlaps = no
break

of mnkPathArray:
# -1 = the arg-block end
# -2 = the arg node
# -3 = the operand
overlaps = sameIndex(body[a - 3], body[b - 3])
if overlaps == no:
break

of ArgumentNodes:
# this happens when invoking a path operator with more than one
# operand. Skip to the end of the arg-block
a = parentEnd(body, a)
b = parentEnd(body, b)
else:
unreachable(an.kind)

inc a
inc b

result = CmpLocsResult(endA: a > lastA, endB: b > lastB, overlaps: overlaps)

func isAPartOfB*(r: CmpLocsResult): Ternary {.inline.} =
result = r.overlaps
if result != no and not r.endB and r.endA:
# B either is or maybe is a part of A, but A is not a part of B
result = no

func isBPartOfA*(r: CmpLocsResult): Ternary {.inline.} =
result = r.overlaps
if result != no and not r.endA and r.endB:
result = no

func isSame*(r: CmpLocsResult): bool {.inline.} =
## Returns whether A and B refer to the exact same location
## (ignoring the type)
result = r.overlaps == yes and r.endA and r.endB

template overlaps*(r: CmpLocsResult): Ternary =
r.overlaps

func compareLvalues*(tree: MirTree, ea, eb: LvalueExpr): CmpLocsResult {.inline.} =
result = compareLvalues(tree, ea.root, eb.root, ea.last, eb.last)

func overlaps*(tree: MirTree, ea, eb: LvalueExpr): Ternary {.inline.} =
## Convenience wrapper
compareLvalues(tree, ea.root, eb.root, ea.last, eb.last).overlaps

func isPartOf*(tree: MirTree, ea, eb: LvalueExpr): Ternary {.inline.} =
## Computes if the location named by `ea` is part of `eb`. Also evaluates to
## 'yes' if both name the exact same location
let cmp = compareLvalues(tree, ea.root, eb.root, ea.last, eb.last)
result = isAPartOfB(cmp)
Loading

0 comments on commit 6849732

Please sign in to comment.