Skip to content

WIP: Implementing Symbolic ASR pass #2200

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

Closed
wants to merge 12 commits into from

Conversation

anutosh491
Copy link
Collaborator

@anutosh491 anutosh491 commented Jul 22, 2023

The pass should be responsible for converting the ASR of Program 1 to Prorgam 2
Program 1

from lpython import S
from sympy import pi

def main0():
    x: S = pi
    print(x)

if __name__ == "__main__":
   main0()

Program 2

from lpython import ccall, CPtr, p_c_pointer, pointer, i64, empty_c_void_p
import os

@ccall(header="symengine/cwrapper.h", c_shared_lib="symengine", c_shared_lib_path=f"{os.environ['CONDA_PREFIX']}/lib")
def basic_new_stack(x: CPtr) -> None:
    pass

@ccall(header="symengine/cwrapper.h", c_shared_lib="symengine", c_shared_lib_path=f"{os.environ['CONDA_PREFIX']}/lib")
def basic_const_pi(x: CPtr) -> None:
    pass

@ccall(header="symengine/cwrapper.h", c_shared_lib="symengine", c_shared_lib_path=f"{os.environ['CONDA_PREFIX']}/lib")
def basic_str(x: CPtr) -> str:
    pass

def main0():
    y: i64 = i64(0)
    x: CPtr = empty_c_void_p()
    p_c_pointer(pointer(y, i64), x)
    basic_new_stack(x)
    basic_const_pi(x)
    print(basic_str(x))

if __name__ == "__main__":
   main0()

@anutosh491 anutosh491 marked this pull request as draft July 22, 2023 06:55
@certik
Copy link
Contributor

certik commented Jul 22, 2023

Very good, great plan.

for (auto &item : current_scope->get_scope()) {
if (is_a<ASR::Variable_t>(*item.second)) {
this->visit_symbol(*item.second);
if(symbolic_replaces_with_CPtr_Function){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's do this operation in the visit_variable itself.
And remove both visit_Module and visit_Function.
I think PassVisitor handles everything, Even transform_stmts.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can access the Function scope using current_scope and the Module scope using current_scope->parent.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Store all the statements in pass_result.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok so if we get rid of visit_Module and visit_Function ,
the scope for function is x.m_parent_symtab and that of module is x.m_parent_symtab->parent right ?

Copy link
Collaborator

@Thirumalai-Shaktivel Thirumalai-Shaktivel Jul 26, 2023

Choose a reason for hiding this comment

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

Yup, that would work, but let's use current_scope instead.
PassVisitor would have assigned current_scope with x.m_symtab in visit_Function (in asr.h: ASRPassBaseWalkVisitor).
So you can use current_scope to access the Function scope and current_scope-> parent to access the Module scope.

Comment on lines +21 to +22
ReplaceSymbolicVisitor(Allocator &al_) :
PassVisitor(al_, nullptr) {
Copy link
Collaborator

@Thirumalai-Shaktivel Thirumalai-Shaktivel Jul 26, 2023

Choose a reason for hiding this comment

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

If this: #2200 (comment) doesn't work, pass m_global_scope as an argument to ReplaceSymbolicVisitor and pass that symtab to PassVisitor instead of nullptr.

@certik
Copy link
Contributor

certik commented Jul 26, 2023

Thanks @Thirumalai-Shaktivel for the review!

@Thirumalai-Shaktivel
Copy link
Collaborator

That's it!
Everything looks good to me, Add a test and Fix the CI failure!

Comment on lines +59 to +60
SymbolTable* current_scope_copy = current_scope;
current_scope = xx.m_parent_symtab;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this, as we already assign it in the Function visitor!

@Thirumalai-Shaktivel
Copy link
Collaborator

Thirumalai-Shaktivel commented Jul 31, 2023

Great job, @anutosh491!
LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants