Skip to content

Add c++20 kernel #77

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 1 commit into from
May 13, 2024
Merged

Conversation

mcbarton
Copy link
Collaborator

The purpose of this PR is to add a c++20 kernel.

@codecov-commenter
Copy link

codecov-commenter commented May 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 52.42%. Comparing base (d203fdf) to head (dd506c4).
Report is 13 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #77      +/-   ##
==========================================
+ Coverage   51.60%   52.42%   +0.81%     
==========================================
  Files          15       15              
  Lines         593      599       +6     
==========================================
+ Hits          306      314       +8     
+ Misses        287      285       -2     

see 1 file with indirect coverage changes

see 1 file with indirect coverage changes

CMakeLists.txt Outdated
@@ -154,6 +154,8 @@ configure_kernel("share/jupyter/kernels/xcpp")
configure_kernel("share/jupyter/kernels/xcpp11")
configure_kernel("share/jupyter/kernels/xcpp14")
configure_kernel("share/jupyter/kernels/xcpp17")
configure_kernel("share/jupyter/kernels/xcpp17-omp")
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure I understand how this works. We need to pass somehow -fopenmp as a flag. XEUS_CPP_OMP probably does not expand to anything...

Copy link
Collaborator Author

@mcbarton mcbarton May 12, 2024

Choose a reason for hiding this comment

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

I'll remove adding the omp-17 kernel and move that to a different PR. I assumed that infrastructure was there to use it, but had accidentally been missed. I'll leave this PR to just adding the c++20 kernel.

Copy link
Collaborator Author

@mcbarton mcbarton May 12, 2024

Choose a reason for hiding this comment

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

I'll remove adding the omp-17 kernel and move that to a different PR. I assumed that infrastructure was there to use it, but had accidentally been missed. I'll leave this PR to just adding the c++20 kernel.

@vgvassilev I've made this change now, so the PR should hopefully be ok to merge now.

@mcbarton mcbarton force-pushed the Add-c++20-kernel branch from d1b646c to dd506c4 Compare May 12, 2024 13:15
Copy link
Contributor

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

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

LGTM. I'd wait for @JohanMabille.

@JohanMabille JohanMabille merged commit b8c94f9 into compiler-research:main May 13, 2024
6 checks passed
@mcbarton mcbarton deleted the Add-c++20-kernel branch May 22, 2024 16:11
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.

5 participants