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

pass_wrap_global_stmts always creates function of abiType::BindC #2700

Closed
wants to merge 3 commits into from

Conversation

Vipul-Cariappa
Copy link
Contributor

Reference #2698 (comment)

@Vipul-Cariappa
Copy link
Contributor Author

cc @Shaikh-Ubaid

@Shaikh-Ubaid
Copy link
Collaborator

This is fine. Can you submit this change to LFortran? So that we know it works with LFortran.

@Shaikh-Ubaid
Copy link
Collaborator

When you make a PR there, just post a link to it here.

@Vipul-Cariappa Vipul-Cariappa changed the title pass_wrap_global_stmts always creates function of abiType::Source pass_wrap_global_stmts always creates function of abiType::BindC May 13, 2024
@Vipul-Cariappa
Copy link
Contributor Author

I initially set the ABI to Source. LFortran does not like Source and tests fail there. So, I instead changed it to BindC. All tests should pass in both LPython and LFortran according to me.
It does not matter what ABI the pass_wrap_global_stmts uses, but I want it to be of the same ABI irrespective of other factors. Because at the LLVM backend, it changes the function name according to the ABI used as explained earlier.

@Vipul-Cariappa
Copy link
Contributor Author

Now, WASM related tests are failing.
I will need to look into it.

@Vipul-Cariappa Vipul-Cariappa marked this pull request as draft May 13, 2024 12:42
@Vipul-Cariappa Vipul-Cariappa marked this pull request as ready for review May 13, 2024 14:42
@Shaikh-Ubaid
Copy link
Collaborator

I initially set the ABI to Source. LFortran does not like Source and tests fail there. So, I instead changed it to BindC

I think changing to Source is fine. But changing to BindC might not be. When we say BindC, it means that the function is implemented in C. For this function, we only know the prototype of the function and the function body is empty.

I think in our case, we have the function body so marking it as BindC only to avoid name mangle does not look right. @Vipul-Cariappa Since the name issue for you arises during interactive compilation, can you trying not mangling name when compiler_options.interactive = true in the llvm backend?

@Vipul-Cariappa
Copy link
Contributor Author

That might be possible. Changing ASRToLLVMVisitor.instantiate_function to avoid name mangling while compiler_options.interactive = true is set.

@Vipul-Cariappa
Copy link
Contributor Author

I guess we can close this as we have merged #2701.

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.

2 participants