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

BindPython ABI #2796

Merged
merged 21 commits into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
272889a
importing `cpython_bindings.py` when using `BindPython` ABI
Vipul-Cariappa Jul 29, 2024
51e3975
BindPython for no args - no return functions in LLVM backend
Vipul-Cariappa Aug 1, 2024
67853c7
BindPython support for args of int and float types
Vipul-Cariappa Aug 2, 2024
94fa4b6
refactored "from ... import ..." AST to ASR code
Vipul-Cariappa Aug 3, 2024
f256e43
BindPython support for args of str and bool types
Vipul-Cariappa Aug 3, 2024
57b0e80
BindPython support for return type of str, bool, integer & real types
Vipul-Cariappa Aug 3, 2024
ff32b75
refactored python_bind
Vipul-Cariappa Aug 3, 2024
42565b5
fix for CI
Vipul-Cariappa Aug 3, 2024
31f1225
fix for failing test
Vipul-Cariappa Aug 4, 2024
62ae095
add integration test for llvm backend
Vipul-Cariappa Aug 4, 2024
24f52fa
refactor: importing cpython_bindings separate out into function
Vipul-Cariappa Aug 4, 2024
3d1b50e
skip python_bind ASR pass when using C backend
Vipul-Cariappa Aug 5, 2024
a3bca79
changes according to code review
Vipul-Cariappa Aug 10, 2024
3cf7b22
generating CPython related function declarations in python_bind pass
Vipul-Cariappa Aug 11, 2024
432b184
fix for failing CI
Vipul-Cariappa Aug 11, 2024
dd6a76b
remove use of `PyRun_SimpleString` to set python path
Vipul-Cariappa Aug 11, 2024
8278e43
clean up unwanted comment
Vipul-Cariappa Aug 11, 2024
644a453
refactored `declare_functions` to asr_utils.cpp
Vipul-Cariappa Aug 11, 2024
e750a04
Update src/libasr/pass/python_bind.cpp
Vipul-Cariappa Aug 12, 2024
9ea5cdc
skipping python_bind ASR pass if `--enable-cpython` flag not used
Vipul-Cariappa Aug 12, 2024
839f6b9
fix related to previous commit
Vipul-Cariappa Aug 12, 2024
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
2 changes: 1 addition & 1 deletion integration_tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,7 @@ RUN(NAME bindc_05 LABELS llvm c
EXTRAFILES bindc_05b.c)
RUN(NAME bindc_06 LABELS llvm c
EXTRAFILES bindc_06b.c)
RUN(NAME bindpy_01 LABELS cpython c_py EXTRA_ARGS --enable-cpython NOFAST COPY_TO_BIN bindpy_01_module.py)
RUN(NAME bindpy_01 LABELS cpython llvm_py c_py EXTRA_ARGS --enable-cpython NOFAST COPY_TO_BIN bindpy_01_module.py)
RUN(NAME bindpy_02 LABELS cpython c_py EXTRA_ARGS --link-numpy COPY_TO_BIN bindpy_02_module.py)
RUN(NAME bindpy_03 LABELS cpython c_py EXTRA_ARGS --link-numpy NOFAST COPY_TO_BIN bindpy_03_module.py)
RUN(NAME bindpy_04 LABELS cpython c_py EXTRA_ARGS --link-numpy NOFAST COPY_TO_BIN bindpy_04_module.py)
Expand Down
1 change: 1 addition & 0 deletions src/bin/lpython.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ int emit_c(const std::string &infile,
pass_manager.use_default_passes(true);
compiler_options.po.always_run = true;
compiler_options.po.run_fun = "f";
compiler_options.po.c_backend = true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

We already have --backend=c which means the backend is c. Another c_backend could seem confusing. I think we should call it something like c_skip_bindpy_pass.

We should also enable this flag in emit_c_to_file().


pass_manager.apply_passes(al, asr, compiler_options.po, diagnostics);

Expand Down
1 change: 1 addition & 0 deletions src/libasr/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ set(SRC
pass/for_all.cpp
pass/while_else.cpp
pass/global_stmts.cpp
pass/python_bind.cpp
pass/select_case.cpp
pass/init_expr.cpp
pass/implied_do_loops.cpp
Expand Down
3 changes: 3 additions & 0 deletions src/libasr/pass/pass_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
#include <libasr/pass/replace_print_struct_type.h>
#include <libasr/pass/promote_allocatable_to_nonallocatable.h>
#include <libasr/pass/replace_function_call_in_declaration.h>
#include <libasr/pass/python_bind.h>
#include <libasr/codegen/asr_to_fortran.h>
#include <libasr/asr_verify.h>
#include <libasr/pickle.h>
Expand All @@ -79,6 +80,7 @@ namespace LCompilers {
{"do_loops", &pass_replace_do_loops},
{"while_else", &pass_while_else},
{"global_stmts", &pass_wrap_global_stmts},
{"python_bind", &pass_python_bind},
{"implied_do_loops", &pass_replace_implied_do_loops},
{"array_op", &pass_replace_array_op},
{"symbolic", &pass_replace_symbolic},
Expand Down Expand Up @@ -206,6 +208,7 @@ namespace LCompilers {
PassManager(): apply_default_passes{false},
c_skip_pass{false} {
_passes = {
"python_bind",
"nested_vars",
"global_stmts",
"transform_optional_argument_functions",
Expand Down
406 changes: 406 additions & 0 deletions src/libasr/pass/python_bind.cpp

Large diffs are not rendered by default.

14 changes: 14 additions & 0 deletions src/libasr/pass/python_bind.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#ifndef LIBASR_PASS_PYTHON_BIND_H
#define LIBASR_PASS_PYTHON_BIND_H

#include <libasr/asr.h>
#include <libasr/utils.h>

namespace LCompilers {

void pass_python_bind(Allocator &al, ASR::TranslationUnit_t &unit,
const PassOptions &pass_options);

} // namespace LCompilers

#endif // LIBASR_PASS_PYTHON_BIND_H
1 change: 1 addition & 0 deletions src/libasr/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ struct PassOptions {
bool tree = false;
bool with_intrinsic_mods = false;
bool c_mangling = false;
bool c_backend = false;
};

struct CompilerOptions {
Expand Down
57 changes: 57 additions & 0 deletions src/lpython/semantics/python_ast_to_asr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4248,6 +4248,7 @@ class SymbolTableVisitor : public CommonVisitor<SymbolTableVisitor> {
bool is_interface = false;
std::string interface_name = "";
bool is_derived_type = false;
bool contains_bindpython = false;
std::vector<std::string> current_procedure_args;
ASR::abiType current_procedure_abi_type = ASR::abiType::Source;
std::map<SymbolTable*, ASR::accessType> assgn;
Expand Down Expand Up @@ -4428,6 +4429,55 @@ class SymbolTableVisitor : public CommonVisitor<SymbolTableVisitor> {
//Implemented in BodyVisitor
}

void import_cpython(const AST::FunctionDef_t &x, SymbolTable *parent_scope) {
std::vector<std::string> pybind_funcs = {
"Py_Initialize",
"Py_IsInitialized",
"PyRun_SimpleString",
"Py_DecodeLocale",
"PySys_SetArgv",
"Py_FinalizeEx",
"PyUnicode_FromString",
"PyUnicode_AsUTF8AndSize",
"PyImport_Import",
"Py_DecRef",
"Py_IncRef",
"PyObject_GetAttrString",
"PyTuple_New",
"PyTuple_SetItem",
"PyObject_CallObject",
"PyLong_AsLongLong",
"PyLong_AsUnsignedLongLong",
"PyLong_FromLongLong",
"PyLong_FromUnsignedLongLong",
"PyFloat_FromDouble",
"PyFloat_AsDouble",
"PyBool_FromLong",
"PyObject_IsTrue",
};
Str s;
AST::alias_t *module_symbols =
al.allocate<AST::alias_t>(pybind_funcs.size());

for (size_t i = 0; i < pybind_funcs.size(); i++) {
s.from_str(al, pybind_funcs.at(i));
(module_symbols + i)->loc = x.base.base.loc;
(module_symbols + i)->m_name = s.c_str(al);
(module_symbols + i)->m_asname = nullptr;
}

AST::ImportFrom_t *imports =
(AST::ImportFrom_t*)AST::make_ImportFrom_t(
al, x.base.base.loc,
(char*)"cpython_bindings", module_symbols,
pybind_funcs.size(), 0);

SymbolTable *function_scope = current_scope;
current_scope = parent_scope;
visit_ImportFrom(*imports);
current_scope = function_scope;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these needed in AST->ASR?

Ideally the initial ASR has nothing to do with Python C/API. Then the ASR->ASR pass translates bind(python) into C/API calls in ASR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Exactly, even I was confused in my last meet with Vipul. Looking into the changes, I think I understand what he is doing here.

He has a new runtime module cpython_bindings.py which contains the declarations to the C functions in Python.h that we need. In the above change, he is just creating AST code for the following:

from cpython_bindings import Py_Initialize, Py_IsInitialized, PyRun_SimpleString, Py_DecodeLocale, PySys_SetArgv, Py_FinalizeEx, PyUnicode_FromString, PyUnicode_AsUTF8AndSize, PyImport_Import, Py_DecRef, Py_IncRef, PyObject_GetAttrString, PyTuple_New, PyTuple_SetItem, PyObject_CallObject, PyLong_AsLongLong, PyLong_AsUnsignedLongLong, PyLong_FromLongLong, PyLong_FromUnsignedLongLong, PyFloat_FromDouble, PyFloat_AsDouble, PyBool_FromLong, PyObject_IsTrue

When he visits the AST code, lpython reads/loads the runtime/cpython_bindings.py and automatically generates the declarations for the above BindC functions.

@Vipul-Cariappa Initially this approach sounded interesting to me, but now after going through the changes, it seems like we should instead write the ASR code for these directly in the python_bind pass. I think if we simply extend the current pybind_funcs array with the parameter types and return type info (for example specifying types like here

m_wa.define_func({i64}, {}, {i64, i64, i64, i64}, "print_i64", [&](){
), we should be able to run a loop and generate the ASR nodes for declaring these. Then, we ideally would not be making any changes in the AST->ASR level. And we can also remove the runtime/cpython_bindings.py file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. I will update the code to generate the required function declarations in the python_bind pass.

Copy link
Contributor

Choose a reason for hiding this comment

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

Initially this approach sounded interesting to me, but now after going through the changes, it seems like we should instead write the ASR code for these directly in the python_bind pass.

Yes, exactly.


void visit_FunctionDef(const AST::FunctionDef_t &x) {
dependencies.clear(al);
SymbolTable *parent_scope = current_scope;
Expand Down Expand Up @@ -4489,6 +4539,13 @@ class SymbolTableVisitor : public CommonVisitor<SymbolTableVisitor> {
current_procedure_abi_type = ASR::abiType::BindPython;
current_procedure_interface = true;
module_file = extract_keyword_val_from_decorator(call_d, "module");

if (!contains_bindpython) {

import_cpython(x, parent_scope);
}
contains_bindpython = true;

} else {
throw SemanticError("Unsupported Decorator type",
x.base.base.loc);
Expand Down
93 changes: 93 additions & 0 deletions src/runtime/cpython_bindings.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
from lpython import ccall, Pointer, i32, i64, u64, f64, empty_c_void_p, CPtr, pointer

@ccall(header="Python.h")
def Py_Initialize():
pass

@ccall(header="Python.h")
def Py_IsInitialized() -> i32:
pass

@ccall(header="Python.h")
def PyRun_SimpleString(s: str) -> i32:
pass

@ccall(header="Python.h")
def Py_DecodeLocale(s: str, p: CPtr) -> CPtr:
pass

@ccall(header="Python.h")
def PySys_SetArgv(n: i32, args: Pointer[CPtr]):
pass

@ccall(header="Python.h")
def Py_FinalizeEx() -> i32:
pass

@ccall(header="Python.h")
def PyUnicode_FromString(s: str) -> CPtr:
pass

@ccall(header="Python.h")
def PyUnicode_AsUTF8AndSize(s: CPtr, l: CPtr) -> str:
pass

@ccall(header="Python.h")
def PyImport_Import(name: CPtr) -> CPtr:
pass

@ccall(header="Python.h")
def Py_DecRef(name: CPtr):
pass

@ccall(header="Python.h")
def Py_IncRef(name: CPtr):
pass

@ccall(header="Python.h")
def PyObject_GetAttrString(m: CPtr, s: str) -> CPtr:
pass

@ccall(header="Python.h")
def PyTuple_New(n: i32) -> CPtr:
pass

@ccall(header="Python.h")
def PyTuple_SetItem(t: CPtr, p: i32, o: CPtr) -> i32:
pass

@ccall(header="Python.h")
def PyObject_CallObject(a: CPtr, b: CPtr) -> CPtr:
pass

@ccall(header="Python.h")
def PyLong_AsLongLong(a: CPtr) -> i64:
pass

@ccall(header="Python.h")
def PyLong_AsUnsignedLongLong(a: CPtr) -> u64:
pass

@ccall(header="Python.h")
def PyLong_FromLongLong(a: i64) -> CPtr:
pass

@ccall(header="Python.h")
def PyLong_FromUnsignedLongLong(a: u64) -> CPtr:
pass

@ccall(header="Python.h")
def PyFloat_FromDouble(a: f64) -> CPtr:
pass

@ccall(header="Python.h")
def PyFloat_AsDouble(a: CPtr) -> f64:
pass

@ccall(header="Python.h")
def PyBool_FromLong(a: i32) -> CPtr:
pass

@ccall(header="Python.h")
def PyObject_IsTrue(a: CPtr) -> i32:
pass
Loading