Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

internal: separate TLoc from TSym and TType #790

Merged
merged 8 commits into from
Jul 9, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 9 additions & 12 deletions compiler/ast/ast.nim
Original file line number Diff line number Diff line change
Expand Up @@ -309,14 +309,6 @@ proc newType*(kind: TTypeKind, id: ItemId; owner: PSym): PType =
echo "KNID ", kind
writeStackTrace()


proc mergeLoc(a: var TLoc, b: TLoc) =
if a.k == low(typeof(a.k)): a.k = b.k
if a.storage == low(typeof(a.storage)): a.storage = b.storage
a.flags.incl b.flags
if a.lode == nil: a.lode = b.lode
if a.r.len == 0: a.r = b.r

proc newSons*(father: Indexable, length: int) =
setLen(father.sons, length)

Expand All @@ -333,7 +325,9 @@ proc assignType*(dest, src: PType) =
if dest.sym != nil:
dest.sym.flags.incl src.sym.flags-{sfExported}
if dest.sym.annex == nil: dest.sym.annex = src.sym.annex
mergeLoc(dest.sym.loc, src.sym.loc)
dest.sym.locFlags.incl src.sym.locFlags
if dest.sym.extname.len == 0:
dest.sym.extname = src.sym.extname
else:
dest.sym = src.sym
newSons(dest, src.len)
Expand All @@ -355,7 +349,8 @@ proc copySym*(s: PSym; id: ItemId): PSym =
result.magic = s.magic
result.options = s.options
result.position = s.position
result.loc = s.loc
result.extname = s.extname
result.locFlags = s.locFlags
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

too big a change to name it extFlags?

a soft suggestion as a fair bit of this is under heavy transition.

result.annex = s.annex # BUGFIX
result.constraint = s.constraint
if result.kind in {skVar, skLet, skField}:
Expand All @@ -372,7 +367,8 @@ proc createModuleAlias*(s: PSym, id: ItemId, newIdent: PIdent, info: TLineInfo;
result.flags = s.flags
result.options = s.options
result.position = s.position
result.loc = s.loc
result.extname = s.extname
result.locFlags = s.locFlags
result.annex = s.annex

proc initStrTable*(x: var TStrTable) =
Expand Down Expand Up @@ -510,7 +506,8 @@ template transitionSymKindCommon*(k: TSymKind) =
s[] = TSym(kind: k, itemId: obj.itemId, magic: obj.magic, typ: obj.typ, name: obj.name,
info: obj.info, owner: obj.owner, flags: obj.flags, ast: obj.ast,
options: obj.options, position: obj.position, offset: obj.offset,
loc: obj.loc, annex: obj.annex, constraint: obj.constraint)
extname: obj.extname, locFlags: obj.locFlags, locId: obj.locId,
annex: obj.annex, constraint: obj.constraint)
when defined(nimsuggest):
s.allUsages = obj.allUsages

Expand Down
39 changes: 13 additions & 26 deletions compiler/ast/ast_types.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1590,19 +1590,13 @@ type
data*: seq[PSym]

# -------------- backend information -------------------------------
TLocKind* = enum
locNone, ## no location
locTemp, ## temporary location
locLocalVar, ## location is a local variable
locGlobalVar, ## location is a global variable
locParam, ## location is a parameter
locField, ## location is a record field
locExpr, ## "location" is really an expression
locProc, ## location is a proc (an address of a procedure)
locData, ## location is a constant
locCall, ## location is a call expression
locOther ## location is something other
TLocFlag* = enum
# XXX: `TLocFlag` conflates two things:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, even more reason to hold off on extFlags until these have been separated.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I'm going to split the enum up immediately after this PR.

# - flags regarding the external interface (e.g., `lfHeader`; set by
# sem, used by sem and the code generators)
# - location-related flags (e.g., ``lfIndirect``, ``lfSingleUse``,
# etc.; only relevant to the C code generator)
# Split the enum up.
lfIndirect, ## backend introduced a pointer
lfFullExternalName, ## only used when 'conf.cmd == cmdNimfix': Indicates
## that the symbol has been imported via 'importc: "fullname"' and
Expand All @@ -1617,19 +1611,8 @@ type
## ptr array due to C array limitations.
## See #1181, #6422, #11171
lfPrepareForMutation ## string location is about to be mutated (V2)
TStorageLoc* = enum
OnUnknown, ## location is unknown (stack, heap or static)
OnStatic, ## in a static section
OnStack, ## location is on hardware stack
OnHeap ## location is on heap or global
## (reference counting needed)

TLocFlags* = set[TLocFlag]
TLoc* = object
k*: TLocKind ## kind of location
storage*: TStorageLoc
flags*: TLocFlags ## location's flags
lode*: PNode ## Node where the location came from; can be faked
r*: Rope ## rope value of location (code generators)

# ---------------- end of backend information ------------------------------

Expand Down Expand Up @@ -1713,7 +1696,12 @@ type
## to the module's fileIdx
## for variables a slot index for the evaluator
offset*: int ## offset of record field
loc*: TLoc
extname*: string ## the external name of the type, or empty if a
## generated name is to be used
locFlags*: TLocFlags ## additional flags that are relevant to code
## generation
locId*: uint32 ## associates the symbol with a loc in the C code
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If two symbols can't be associated with the same location, could we piggy back off of itemId?

(I'm guessing this will result in a 'no' once I've read further in the PR)

It would mean we'd have to store something for each symbol (more memory), but that might balance out as we're storing an extra 4 bytes with the current change. The lack of loc information would have to be tracked on the loc data end, which does mean a deref/index-lookup prior to knowing it doesn't exist.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revisiting this comment, I'm pretty sure this wouldn't be a good approach at present, better to defer it as the necessary data and consequent layout are likely to change dramatically.

## generator. 0 means unset.
annex*: PLib ## additional fields (seldom used, so we use a
## reference to another object to save space)
constraint*: PNode ## additional constraints like 'lit|result'; also
Expand Down Expand Up @@ -1759,7 +1747,6 @@ type
align*: int16 ## the type's alignment requirements
paddingAtEnd*: int16 ##
lockLevel*: TLockLevel ## lock level as required for deadlock checking
loc*: TLoc
typeInst*: PType ## for generic instantiations the tyGenericInst that led to this
## type; for tyError the previous type if avaiable
uniqueId*: ItemId ## due to a design mistake, we need to keep the real ID here as it
Expand Down
14 changes: 7 additions & 7 deletions compiler/ast/ndi.nim
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,19 @@ type
f: File
buf: string
filename: AbsoluteFile
syms: seq[PSym]
syms: seq[tuple[s: PSym, name: string]]

proc doWrite(f: var NdiFile; s: PSym; conf: ConfigRef) =
proc doWrite(f: var NdiFile; s: PSym; name: string, conf: ConfigRef) =
f.buf.setLen 0
f.buf.addInt s.info.line.int
f.buf.add "\t"
f.buf.addInt s.info.col.int
f.f.write(s.name.s, "\t")
f.f.writeRope(s.loc.r)
f.f.writeRope(name)
f.f.writeLine("\t", toFullPath(conf, s.info), "\t", f.buf)

template writeMangledName*(f: NdiFile; s: PSym; conf: ConfigRef) =
if f.enabled: f.syms.add s
template writeMangledName*(f: NdiFile; s: PSym; name: string, conf: ConfigRef) =
if f.enabled: f.syms.add (s, name)

proc open*(f: var NdiFile; filename: AbsoluteFile; conf: ConfigRef) =
f.enabled = not filename.isEmpty
Expand All @@ -53,8 +53,8 @@ proc close*(f: var NdiFile, conf: ConfigRef) =
if f.enabled:
f.f = open(f.filename.string, fmWrite, 8000)
doAssert f.f != nil, f.filename.string
for s in f.syms:
doWrite(f, s, conf)
for (s, name) in f.syms:
doWrite(f, s, name, conf)
close(f.f)
f.syms.reset
f.filename.reset
6 changes: 3 additions & 3 deletions compiler/backend/backends.nim
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ func discoverFrom*(data: var DiscoveryData, decl: PNode) =
of nkProcDef, nkFuncDef, nkConverterDef, nkMethodDef:
let prc = n[namePos].sym
if {sfExportc, sfCompilerProc} * prc.flags == {sfExportc} or
(sfExportc in prc.flags and lfExportLib in prc.loc.flags):
(sfExportc in prc.flags and lfExportLib in prc.locFlags):
# an exported routine. It must always have code generated for it. Note
# that compilerprocs, while exported, are still only have code generated
# for them when used
Expand Down Expand Up @@ -524,7 +524,7 @@ func queue(iter: var ProcedureIter, prc: PSym, m: FileIndex) =
## If eligible for processing and code generation, adds `prc` to
## `iter`'s queue.
assert prc.kind in routineKinds
if lfNoDecl notin prc.loc.flags and
if lfNoDecl notin prc.locFlags and
(sfImportc notin prc.flags or (iter.config.noImported and
prc.ast[bodyPos].kind != nkEmpty)):
iter.queued.add (prc, m)
Expand Down Expand Up @@ -564,7 +564,7 @@ proc preprocessDynlib(graph: ModuleGraph, idgen: IdGenerator,
# horrendous, but fortunately, this hack (`preprocessDynlib``) can
# be removed once handling of dynlib procedures and globals is fully
# implemented in the ``process`` iterator
if lfDynamicLib in sym.loc.flags:
if lfDynamicLib in sym.locFlags:
if sym.annex.path.kind in nkStrKinds:
# it's a string, no need to transform nor scan it
discard
Expand Down
48 changes: 43 additions & 5 deletions compiler/backend/cbackend.nim
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ import
],
compiler/ast/[
ast,
lineinfos
lineinfos,
ndi
],
compiler/backend/[
backends,
Expand Down Expand Up @@ -93,6 +94,14 @@ type

func hash(x: PSym): int = hash(x.id)

proc writeMangledLocals(p: BProc) =
## Writes the mangled names of `p`'s locals to the module's NDI file.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a nice secondary benefit developing over time, as the mangling becomes more and more consistent it's going to make gdb pretty printers' life all that much easier.

for it in p.locals.items:
# writing out mangled names for compiler-inserted variables (temporaries)
# is not necessary (lode is guaranteed to be a symbol)
if it.lode.sym.kind != skTemp:
writeMangledName(p.module.ndi, it.lode.sym, it.r, p.config)

func registerInline(g: var InliningData, prc: PSym): uint32 =
## If not already registered, registers the inline procedure `prc` with
## `g`. This only sets up an ``InlineProc`` stub -- the entry is
Expand Down Expand Up @@ -151,7 +160,7 @@ proc processEvent(g: BModuleList, inl: var InliningData, discovery: var Discover
# XXX: dynlib procedure handling is going to move into the unified backend
# processing pipeline (i.e., the ``process`` iterator) in the future
for _, s in peek(discovery.procedures):
if lfDynamicLib in s.loc.flags:
if lfDynamicLib in s.locFlags:
let m = g.modules[s.itemId.module.int]
fillProcLoc(m, newSymNode(s))
symInDynamicLib(m, s)
Expand Down Expand Up @@ -237,7 +246,13 @@ proc processEvent(g: BModuleList, inl: var InliningData, discovery: var Discover
bmod.declaredThings.incl(evt.sym.id)
let
body = generateAST(g.graph, bmod.idgen, evt.sym, evt.body)
r = genProc(bmod, evt.sym, body)
p = startProc(bmod, evt.sym, body)

# we can't generate with ``genProc`` because we still need to output
# the mangled names
genStmts(p, body)
writeMangledLocals(p)
let r = finishProc(p, evt.sym)

if inlineId.isSome:
# remember the generated body:
Expand Down Expand Up @@ -365,8 +380,8 @@ proc generateCode*(graph: ModuleGraph, g: BModuleList, mlist: sink ModuleList) =

# the init and data-init procedures use special names in the
# generated code:
m.init.loc.r = getInitName(bmod)
m.dataInit.loc.r = getDatInitName(bmod)
m.init.extname = getInitName(bmod)
m.dataInit.extname = getDatInitName(bmod)

# mark the init procedure so that the code generator can detect and
# special-case it:
Expand All @@ -389,12 +404,35 @@ proc generateCode*(graph: ModuleGraph, g: BModuleList, mlist: sink ModuleList) =

# finish the partial procedures:
for s, p in partial.pairs:
writeMangledLocals(p)
p.module.s[cfsProcs].add(finishProc(p, s))

# -------------------------
# all alive entities must have been discovered when reaching here; it is
# not allowed to raise new ones beyond this point

block:
# write the mangled names of the various entities to the NDI files
for it in g.procs.items:
let
s = it.loc.lode.sym
m = g.modules[moduleId(s)]
writeMangledName(m.ndi, s, it.loc.r, g.config)
# parameters:
for p in it.params.items:
if p.k != locNone: # not all parameters have locs
writeMangledName(m.ndi, s, p.r, g.config)

template write(loc: TLoc) =
let s = loc.lode.sym
writeMangledName(g.modules[moduleId(s)].ndi, s, loc.r, g.config)

for it in g.globals.items:
write(it)

for it in g.consts.items:
write(it)

# now emit a duplicate of each inline procedure into the C files where the
# procedure is used. Due to how ``cgen`` currently works, this means
# generating C code for the procedure again
Expand Down
2 changes: 1 addition & 1 deletion compiler/backend/ccgcalls.nim
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ proc genClosureCall(p: BProc, le, ri: PNode, d: var TLoc) =

proc notYetAlive(n: PNode): bool {.inline.} =
let r = getRoot(n)
result = r != nil and r.loc.lode == nil
result = r != nil and r.locId == 0

proc isInactiveDestructorCall(p: BProc, e: PNode): bool =
#[ Consider this example.
Expand Down
Loading