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

Call basic_free_stack before return statement #2411

Closed
Thirumalai-Shaktivel opened this issue Nov 8, 2023 · 8 comments · Fixed by #2412
Closed

Call basic_free_stack before return statement #2411

Thirumalai-Shaktivel opened this issue Nov 8, 2023 · 8 comments · Fixed by #2412
Labels

Comments

@Thirumalai-Shaktivel
Copy link
Collaborator

from lpython import S
from sympy import pi

def func() -> S:
    print(pi)
    return pi

def test_func():
    print(func())

test_func()
(SubroutineCall
    2 basic_const_pi
    2 basic_const_pi
    [((Var 3 _lpython_return_variable))]
    ()
)
(Return)
(SubroutineCall                    ;; <== Here
    2 basic_free_stack
    2 basic_free_stack
    [((Var 3 _lpython_return_variable))]
    ()
)
(SubroutineCall
    2 basic_free_stack
    2 basic_free_stack
    [((Var 3 stack0))]
    ()
)
@certik
Copy link
Contributor

certik commented Nov 9, 2023

Consider this example:

from lpython import S
from sympy import pi, Symbol

def func() -> S:
    return pi

def test_func():
    z: S = func()
    print(z)

test_func()

I recommend to fix it by calling the subroutine_from_function pass, which transforms the above into the following ASR code:

from lpython import S
from sympy import pi, Symbol

def func(r: Out[S]) -> None:
    r = pi

def test_func():
    z: S
    func(z)
    print(z)

test_func()

And this code in turn should already work today, so nothing needs to be implemented in the Symbolic pass.

@certik
Copy link
Contributor

certik commented Nov 9, 2023

The first issue to fix is:

$ lpython a.py --verbose
ASR Pass starts: 'nested_vars'
ASR Pass ends: 'nested_vars'
ASR Pass starts: 'global_stmts'
ASR Pass ends: 'global_stmts'
ASR Pass starts: 'init_expr'
ASR Pass ends: 'init_expr'
ASR Pass starts: 'implied_do_loops'
ASR Pass ends: 'implied_do_loops'
ASR Pass starts: 'class_constructor'
ASR Pass ends: 'class_constructor'
ASR Pass starts: 'pass_list_expr'
ASR Pass ends: 'pass_list_expr'
ASR Pass starts: 'where'
ASR Pass ends: 'where'
ASR Pass starts: 'subroutine_from_function'
ASR Pass ends: 'subroutine_from_function'
ASR Pass starts: 'array_op'
ASR Pass ends: 'array_op'
ASR Pass starts: 'symbolic'
Traceback (most recent call last):
  File "/Users/ondrej/repos/lpython/src/bin/lpython.cpp", line 1869
    err = compile_python_to_object_file(arg_file, tmp_o, runtime_library_dir,
  File "/Users/ondrej/repos/lpython/src/bin/lpython.cpp", line 828
    res = fe.get_llvm3(*asr, pass_manager, diagnostics, infile);
  File "/Users/ondrej/repos/lpython/src/lpython/python_evaluator.cpp", line 71
    run_fn, infile);
  File "/Users/ondrej/repos/lpython/src/libasr/codegen/asr_to_llvm.cpp", line 8925
    pass_manager.apply_passes(al, &asr, pass_options, diagnostics);
  File "/Users/ondrej/repos/lpython/src/libasr/pass/pass_manager.h", line 305
    apply_passes(al, asr, _passes, pass_options, diagnostics);
  File "/Users/ondrej/repos/lpython/src/libasr/pass/pass_manager.h", line 166
    if (!asr_verify(*asr, true, diagnostics)) {
  File "/Users/ondrej/repos/lpython/src/libasr/asr_verify.cpp", line 1161
    v.visit_TranslationUnit(unit);
  File "/Users/ondrej/repos/lpython/src/libasr/asr_verify.cpp", line 109
    for (auto &a : x.m_symtab->get_scope()) {
  File "../libasr/asr.h", line 5040
  File "../libasr/asr.h", line 4755
  File "/Users/ondrej/repos/lpython/src/libasr/asr_verify.cpp", line 317
    for (auto &a : x.m_symtab->get_scope()) {
  File "../libasr/asr.h", line 5040
  File "../libasr/asr.h", line 4756
  File "/Users/ondrej/repos/lpython/src/libasr/asr_verify.cpp", line 435
    visit_stmt(*x.m_body[i]);
  File "../libasr/asr.h", line 5057
  File "../libasr/asr.h", line 4806
  File "/Users/ondrej/repos/lpython/src/libasr/asr_verify.cpp", line 870
    require(symtab_in_scope(current_symtab, x.m_name),
  File "/Users/ondrej/repos/lpython/src/libasr/asr_verify.cpp", line 68
    unsigned int symtab_ID = symbol_parent_symtab(sym)->counter;
  File "/Users/ondrej/repos/lpython/src/libasr/asr_verify.cpp", line 68
    unsigned int symtab_ID = symbol_parent_symtab(sym)->counter;
  Binary file "/usr/lib/system/libsystem_platform.dylib", local address: 0x1803244e3
Segfault: Signal SIGSEGV (segmentation fault) received

@certik
Copy link
Contributor

certik commented Nov 9, 2023

There are several independent issues that we need to fix:

@anutosh491
Copy link
Collaborator

anutosh491 commented Nov 11, 2023

Considering the above comment. I would say this

from lpython import S
from sympy import pi, Symbol

def func(r: Out[S]) -> None:
    r = pi

def test_func():
    z: S
    func(z)
    print(z)

test_func()

Should transform into something like this

def func(r: Out[CPtr]) -> None:
    basic_const_pi(r)

def test_func():
    _z: i64 = i64(0)
    z: CPtr = empty_c_void_p()
    p_c_pointer(pointer(_z, i64), z)
    basic_new_stack(z)
    func(z)
    print(basic_str(z))

test_func()

This works with the c backend

(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine --backend=c integration_tests/symbolics_08.py
pi

But with the llvm backend we face this issue

code generation error: asr_to_llvm: module failed verification. Error:
Call parameter type does not match function signature!
void** %r
 void*  call void @basic_const_pi(void** %r)
Call parameter type does not match function signature!
  %2 = load void*, void** %z, align 8
 void**  call void @__module___main___func(void* %2)

We define basic_const_pi like

@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

But if you see the error message it is passing a double pointer as the first argument. Is this because r has an intent of type Out ?
cc @certik @Thirumalai-Shaktivel

@Thirumalai-Shaktivel
Copy link
Collaborator Author

Thirumalai-Shaktivel commented Nov 11, 2023

Yup, it's because of intent Out.
I don't know how to proceed here; I will have to look into it soon and report back.

@certik certik reopened this Nov 11, 2023
@certik
Copy link
Contributor

certik commented Nov 11, 2023

@anutosh491 yes, correct. I reopened this issue since the comment #2411 (comment) is not fixed. You are on the right track, we first need to get the Out[S] arguments working.

@anutosh491
Copy link
Collaborator

I think we have all 4 TODOS of #2411 (comment) addressed now. Hence the issue can be closed.

@certik
Copy link
Contributor

certik commented Nov 19, 2023

@anutosh491 great job!

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

Successfully merging a pull request may close this issue.

3 participants