dlopen: Progress towards compiling procedure pointers with C backend#27576
Conversation
Remove some old code doing GC of 'FunctionType'. It's not doing what I thought it did and could cause 'null' holes in the 'gTypeSymbols' global vector. I'll worry about GC with '--verify' at a later point in this effort. There is context-sensitive code in 'SymExpr::codegen()' which checks to see how a 'SymExpr' over a 'FnSymbol' is used. If it looks like it's being used as value, we generate it differently by adding it to the procedure pointer cache and producing a wide index. Previously this code was only firing for LLVM. I moved it out into a more "backend agnostic" block of code. I added code to generate a C function type from a 'FunctionType', but currently that code is commented out. I am opting to generate 'wide proc' as 'int64_t' and 'local proc' as 'void*' in order to work around circular dependencies in C (e.g., a record contains a procedure pointer and the procedure pointer accepts that record as a formal). Signed-off-by: David Longnecker <dlongnecke-cray@users.noreply.github.com>
dlongnecke-cray
force-pushed
the
dynamic-loading-10
branch
from
af60353 to
c307ad0
Compare
dlongnecke-cray
changed the title
Signed-off-by: David Longnecker <dlongnecke-cray@users.noreply.github.com>
|
|
||
| // TODO (dlongnecke): The C backend requires a cast to a procedure | ||
| // type in order to call, while the LLVM backend does not. | ||
| if (gGenInfo->cfile) { |
| // And make sure the LLVM type for the function is generated. | ||
| if (chplFnType) chplFnType->codegenDef(); | ||
|
|
There was a problem hiding this comment.
just double checking my understanding, this is no longer needed because we are eagerly generating function types right?
There was a problem hiding this comment.
after reading the rest of the PR, I think this is the case
There was a problem hiding this comment.
Yes, not only that, but in the past we could redundantly generate function types under LLVM because it is idempotent under that backend. EDIT: It still is, but we don't need to redundantly try 😄.
compiler/codegen/cg-type.cpp
Outdated
| static void codegenFunctionTypeWide(FunctionType* ft) { | ||
| if (auto outfile = gGenInfo->cfile) { | ||
| // TODO: Should I get the code-generated 'int(64)' type here instead? | ||
| fprintf(outfile, "typedef %s %s;\n\n", "int64_t", ft->symbol->cname); |
There was a problem hiding this comment.
I think you should solve the TODO
Should be dtInt[INT_SIZE_64]->codegen().c.c_str()
There was a problem hiding this comment.
You could probably shortcut that as dtInt[INT_SIZE_64]->cname
There was a problem hiding this comment.
Yeah, that should work, but the //TODO is mainly there because I'm not really sure what the precedent is. Do we care to do that? Because we don't really seem to do it consistently, we'll just generate int64_t and void* etc in other places. What's your preference here?
There was a problem hiding this comment.
My preference would be an approach that has both precedent and means that "int64_t" only shows up in our code once for codegen. I pulled that snippet from VarSymbol::codegenDef, but as long as there is precedent I am good with it
compiler/codegen/codegen.cpp
Outdated
| GenInfo* info = gGenInfo; | ||
|
|
||
| // Collected when considering both types and 'FnSymbol'. | ||
| std::set<FunctionType*> fnTypesToCodegen; |
There was a problem hiding this comment.
consider std::unordered_set or llvm::DenseSet
There was a problem hiding this comment.
or even llvm::SmallPtrSet, if you expect fnTypesToCodegen to be small
compiler/codegen/codegen.cpp
Outdated
| auto it = fnTypesToCodegen.find(ft); | ||
| if (it == fnTypesToCodegen.end()) { | ||
| // TODO (dlongnecke): Rare case right now, so leave breakpoint... | ||
| debuggerBreakHere(); | ||
| fnTypesToCodegen.emplace_hint(it, ft); |
There was a problem hiding this comment.
nit suggestion
if (auto it = fnTypesToCodegen.find(ft); it == fnTypesToCodegen.end()) {
compiler/codegen/codegen.cpp
Outdated
| auto it = fnTypesToCodegen.find(ft); | ||
| if (it == fnTypesToCodegen.end()) { |
There was a problem hiding this comment.
nit suggestion
if (auto it = fnTypesToCodegen.find(ft); it == fnTypesToCodegen.end()) {
Signed-off-by: David Longnecker <dlongnecke-cray@users.noreply.github.com>
2 participants
This branch adds my initial progress towards compiling procedure pointers with the C backend.
Remove faulty code I wrote to try and do GC of function types. As a consequence they may never be GC'd, but that should not result in leaks because they will also never be removed from the AST. I will focus on pruning them more actively in a future effort.
Rearrange how the compiler collects and generates function types in
codegen_header. An initial pass overgTypeSymbolscollect function types that are used directly (i.e., they have aSymExprreferring to them andisUsed() == true). Later, while collectingFnSymbol*, the types of functions used to construct procedure pointer values will be collected as well.Represent
local procasvoid*inCin order to handle a circular dependency issue withstructvalue types.Write code to convert
FunctionType*to a C function pointer type, but comment it out. It might be useful later.This branch is followed up by #27588.
Reviewed by @jabraham17. Thanks!