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

llvm: Pass EmitOptions to libzigcpp by pointer. #21251

Merged
merged 1 commit into from
Aug 30, 2024

Conversation

alexrp
Copy link
Member

@alexrp alexrp commented Aug 30, 2024

Passing it by value means that bringup on new architectures is harder for no real benefit. Passing it by pointer allows to get the compiler running without needing to figure out the C calling convention details first. This manifested in practice on LoongArch, for example.

See: ziglang/zig-bootstrap#166 (comment)

The intent here is not to sweep the problem under the rug; I have plans to go through and improve our C calling convention conformance across the board. This just makes initial porting easier.

cc @yxd-ym please test this and see if it gets you unblocked.

Passing it by value means that bringup on new architectures is harder for no
real benefit. Passing it by pointer allows to get the compiler running without
needing to figure out the C calling convention details first. This manifested in
practice on LoongArch, for example.
@yxd-ym
Copy link
Contributor

yxd-ym commented Aug 30, 2024

I applied this patch on my tree

https://github.com/yxd-ym/zig-bootstrap/tree/bootstrap-loongarch-2

and now I'm able to bootstrap both loongarch64-linux-gnu and loongarch64-linux-musl targets of zig binary.

Also, these zig binaries of both targets are able to compile simple hello world C program and zig program.

@alexrp alexrp marked this pull request as ready for review August 30, 2024 15:09
@alexrp
Copy link
Member Author

alexrp commented Aug 30, 2024

Great, thanks for testing!

@andrewrk andrewrk merged commit 5723fca into ziglang:master Aug 30, 2024
10 checks passed
@alexrp alexrp deleted the la64-c-abi-workaround branch August 31, 2024 01:18
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