-
Notifications
You must be signed in to change notification settings - Fork 170
[ASRPass] Wrap all the global symbols into a module #1491
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
Changes from all commits
cc5600c
39aed26
b0bd7b8
ccd3cf1
092a46e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
from ltypes import i32 | ||
|
||
x: list[i32] | ||
x = [1, 2] | ||
i: i32 | ||
i = x[0] | ||
|
||
def test_global_symbols(): | ||
assert i == 1 | ||
assert x[1] == 2 | ||
|
||
test_global_symbols() |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -135,4 +135,105 @@ std::string SymbolTable::get_unique_name(const std::string &name) { | |
return unique_name; | ||
} | ||
|
||
void SymbolTable::move_symbols_from_global_scope(Allocator &al, | ||
SymbolTable *module_scope, Vec<char *> &syms, | ||
Vec<char *> &mod_dependencies) { | ||
// TODO: This isn't scalable. We have write a visitor in asdl_cpp.py | ||
syms.reserve(al, 4); | ||
mod_dependencies.reserve(al, 4); | ||
for (auto &a : scope) { | ||
switch (a.second->type) { | ||
case (ASR::symbolType::Module): { | ||
// Pass | ||
break; | ||
} case (ASR::symbolType::Function) : { | ||
ASR::Function_t *fn = ASR::down_cast<ASR::Function_t>(a.second); | ||
for (size_t i = 0; i < fn->n_dependencies; i++ ) { | ||
ASR::symbol_t *s = fn->m_symtab->get_symbol( | ||
fn->m_dependencies[i]); | ||
if (s == nullptr) { | ||
std::string block_name = "block"; | ||
ASR::symbol_t *block_s = fn->m_symtab->get_symbol(block_name); | ||
int32_t j = 1; | ||
while(block_s != nullptr) { | ||
while(block_s != nullptr) { | ||
ASR::Block_t *b = ASR::down_cast<ASR::Block_t>(block_s); | ||
s = b->m_symtab->get_symbol(fn->m_dependencies[i]); | ||
if (s == nullptr) { | ||
block_s = b->m_symtab->get_symbol("block"); | ||
} else { | ||
break; | ||
} | ||
} | ||
if (s == nullptr) { | ||
block_s = fn->m_symtab->get_symbol(block_name + | ||
std::to_string(j)); | ||
j++; | ||
} else { | ||
break; | ||
} | ||
} | ||
} | ||
if (s == nullptr) { | ||
s = fn->m_symtab->parent->get_symbol(fn->m_dependencies[i]); | ||
} | ||
if (s != nullptr && ASR::is_a<ASR::ExternalSymbol_t>(*s)) { | ||
char *es_name = ASR::down_cast< | ||
ASR::ExternalSymbol_t>(s)->m_module_name; | ||
if (!present(mod_dependencies, es_name)) { | ||
mod_dependencies.push_back(al, es_name); | ||
} | ||
} | ||
} | ||
fn->m_symtab->parent = module_scope; | ||
module_scope->add_symbol(a.first, (ASR::symbol_t *) fn); | ||
syms.push_back(al, s2c(al, a.first)); | ||
break; | ||
} case (ASR::symbolType::GenericProcedure) : { | ||
ASR::GenericProcedure_t *es = ASR::down_cast<ASR::GenericProcedure_t>(a.second); | ||
es->m_parent_symtab = module_scope; | ||
module_scope->add_symbol(a.first, (ASR::symbol_t *) es); | ||
syms.push_back(al, s2c(al, a.first)); | ||
break; | ||
} case (ASR::symbolType::ExternalSymbol) : { | ||
ASR::ExternalSymbol_t *es = ASR::down_cast<ASR::ExternalSymbol_t>(a.second); | ||
if (!present(mod_dependencies, es->m_module_name)) { | ||
mod_dependencies.push_back(al, es->m_module_name); | ||
} | ||
es->m_parent_symtab = module_scope; | ||
module_scope->add_symbol(a.first, (ASR::symbol_t *) es); | ||
syms.push_back(al, s2c(al, a.first)); | ||
break; | ||
} case (ASR::symbolType::StructType) : { | ||
ASR::StructType_t *st = ASR::down_cast<ASR::StructType_t>(a.second); | ||
st->m_symtab->parent = module_scope; | ||
module_scope->add_symbol(a.first, (ASR::symbol_t *) st); | ||
syms.push_back(al, s2c(al, a.first)); | ||
break; | ||
} case (ASR::symbolType::EnumType) : { | ||
ASR::EnumType_t *et = ASR::down_cast<ASR::EnumType_t>(a.second); | ||
et->m_symtab->parent = module_scope; | ||
module_scope->add_symbol(a.first, (ASR::symbol_t *) et); | ||
syms.push_back(al, s2c(al, a.first)); | ||
break; | ||
} case (ASR::symbolType::UnionType) : { | ||
ASR::UnionType_t *ut = ASR::down_cast<ASR::UnionType_t>(a.second); | ||
ut->m_symtab->parent = module_scope; | ||
module_scope->add_symbol(a.first, (ASR::symbol_t *) ut); | ||
syms.push_back(al, s2c(al, a.first)); | ||
break; | ||
} case (ASR::symbolType::Variable) : { | ||
ASR::Variable_t *v = ASR::down_cast<ASR::Variable_t>(a.second); | ||
v->m_parent_symtab = module_scope; | ||
module_scope->add_symbol(a.first, (ASR::symbol_t *) v); | ||
syms.push_back(al, s2c(al, a.first)); | ||
break; | ||
} default : { | ||
throw LCompilersException("Moving the symbol:`" + a.first + | ||
"` from global scope is not implemented yet"); | ||
}; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't scalable. I would do it by writing a visitor in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added this in TODO |
||
} | ||
} | ||
|
||
} // namespace LCompilers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not
llvm
andc
here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In LLVM, the global lists are not yet handled. After merging this I will start working on it.
C: I didn't try it. How do I verify this works in C?
Using
lpython integration_tests/global_syms_01.py --show-c
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any rough idea on how that will be managed. I think LLVM doesn't allow
builder->CreateAlloca
in global scope. Worth researching approaches (implementation in LLVM is not at all required just a rough idea that there is a possibility to make it work in LLVM) once this before merging this because the success of this might rely on the possibility of actually using it in LLVM.One approach in my mind is to create
void*
typed global variables in the global scope and then bitcast them intolist
LLVM type inside LLVM functions. That way any manipulation will happen inside LLVM functions.Other approach is to see how clang++ does it for global variables like
std::vector
in C++. For example the below code,clang++
's LLVM output,It can be seen that it creates global variables but initialises them inside a function. So you can do something similar for lists as well. Also, worth noting that we will have to create a function something like
global_module_function
in LLVM and then call insidemain
. But that should be doable.If LLVM is not enabled then C is not worth enabling. LLVM should always work with a given ASR design then other backends will automatically follow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am approving it as I think this is clean enough at the ASR as well as LLVM/C level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@czgdp1807 the question here is how to handle module variables, as well as program variables. They are scoped to the module or program, but are essentially global. WASM and x86 have the same issue (@Shaikh-Ubaid and I discussed this a few times). It seems one must allocate the global variable (using the platform dependent mechanism), and then initialize it from the main function as needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So seems like global statements are already being wrapped into a main function in the ASR itself? Well I would like to see what we will need to do when we enable
llvm
for this test. Let’s merge it for now (I have approved earlier today).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me review this first.