Skip to content

List in C/C++ backend #1095

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

Merged
merged 4 commits into from
Sep 23, 2022
Merged

List in C/C++ backend #1095

merged 4 commits into from
Sep 23, 2022

Conversation

czgdp1807
Copy link
Collaborator

@czgdp1807 czgdp1807 commented Sep 7, 2022

Implementing 2.ii in #1077 (comment).

Closes #1077

Benchmark - #857 (comment) (see list04 row). Our generated C code (with no optimisation flags enabled for C compiler) is faster than un-optimised (no -O3 flag) executable generated clang++.

@czgdp1807 czgdp1807 added the c Label for C language related changes label Sep 7, 2022
@czgdp1807 czgdp1807 mentioned this pull request Sep 11, 2022
@czgdp1807 czgdp1807 force-pushed the list04 branch 2 times, most recently from 6d010bd to 3893ed8 Compare September 14, 2022 04:28
@czgdp1807 czgdp1807 marked this pull request as ready for review September 14, 2022 04:38
@czgdp1807 czgdp1807 requested a review from certik September 14, 2022 04:38
@czgdp1807
Copy link
Collaborator Author

@certik This is ready to review. Feel free to review the diff in total and let me know if any change is to be made. Otherwise I will clean up the git history and merge.

Copy link
Contributor

@certik certik left a comment

Choose a reason for hiding this comment

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

Thanks for implementing it!

I think this is fine to merge.

As an improvement in subsequent PRs, the C++ version should just use std::vector for lists, as that is the natural data structure that people would use in C++, pretty much equivalent to our list in ASR.

@czgdp1807
Copy link
Collaborator Author

As an improvement in subsequent PRs, the C++ version should just use std::vector for lists, as that is the natural data structure that people would use in C++, pretty much equivalent to our list in ASR.

I think we should expose this with something like --use-stl for C++ backend. The scene is that our implementation is very competitive and light weight (no templates because we already know the types so we directly create the correctly typed lists and hence we don't depend on templating support of C++). So, it should be left to the user to decide whether they want to use our implementations for container data structures or STL implementations.

@czgdp1807 czgdp1807 enabled auto-merge September 23, 2022 06:08
@czgdp1807 czgdp1807 merged commit 973438f into lcompilers:main Sep 23, 2022
@czgdp1807 czgdp1807 deleted the list04 branch September 23, 2022 06:54
@certik
Copy link
Contributor

certik commented Sep 23, 2022

Yes, making it configurable is even better.

@certik
Copy link
Contributor

certik commented Sep 23, 2022

How do the list benchmarks behave? Are we now getting the same performance in C/C++ as in LLVM, and if not, why not?

@czgdp1807
Copy link
Collaborator Author

I don’t know how to enable -O3 optimisation for C compiler in integration tests. Please let me know and I will share the benchmark data with you.

@certik
Copy link
Contributor

certik commented Sep 23, 2022

I would execute the benchmarks by hand. Just compile it by hand and enable all optimization options.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c Label for C language related changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

list in C++/C backend
2 participants