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

[Fix] missing upstream fixes #82

Merged
merged 3 commits into from
Nov 20, 2024
Merged

[Fix] missing upstream fixes #82

merged 3 commits into from
Nov 20, 2024

Conversation

g5t
Copy link
Collaborator

@g5t g5t commented Nov 18, 2024

No description provided.

@g5t
Copy link
Collaborator Author

g5t commented Nov 18, 2024

@willend I don't have an easy way to test that I've implemented your changes correctly, especially McStasMcXtrace/McCode#1761

Could you possibly use this branch with your testing infrastructure to check if things look right?
artifact.zip
(The actions derived wheel and source distributions are attached, should you need them)

@willend
Copy link
Contributor

willend commented Nov 18, 2024

@g5t thanks, I have installed locally via the .whl which came in very handy!

Current test incoming is for CPU MPI initially (should I try with --openacc / multicore also?)

@g5t
Copy link
Collaborator Author

g5t commented Nov 18, 2024

It's the OPENACC/MULTICORE defines that I'm least confident about, so a test that exercises them would be welcome 😁

@willend
Copy link
Contributor

willend commented Nov 18, 2024

Since I only had mccode-antlr tooling on the CPU-only box, testing with GPU/OPENACC required a little more handheld configuration.... But now an 8-way GPU run is also incoming.

(Sync with upstream top-level cogen defines that control/influence
 the use of __builtin_isnan() and friends.)
@willend
Copy link
Contributor

willend commented Nov 20, 2024

@g5t just added a commit to PR home-branch that adds a little define-hierarchy at the top of the cogen header:

#ifndef WIN32
#  ifndef OPENACC
#    define _GNU_SOURCE
#  endif
#  define _POSIX_C_SOURCE 200809L
#endif

(The _GNU_SOURCE needs to be there to ensure correct use of __builtin_isnan on (also very old libc) gcc-environments like conda, for the Nvidia nvc (openacc) compiler, that define should not be there, and naturally cl.exe understands none of it all ;-) )

With this, most of the differences in compilation success between "CPU-antlr" and "GPU-antlr" seen in https://new-nightly.mcstas.org/2024-11-18_output/2024-11-18_output.html (col 2-3 vs 4) should go away...

Copy link
Contributor

@willend willend left a comment

Choose a reason for hiding this comment

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

As mentioned on the PR itself, I just a commit to PR home-branch that adds a little define-hierarchy at the top of the cogen header:

#ifndef WIN32
#  ifndef OPENACC
#    define _GNU_SOURCE
#  endif
#  define _POSIX_C_SOURCE 200809L
#endif

(The _GNU_SOURCE needs to be there to ensure correct use of __builtin_isnan on (also very old libc) gcc-environments like conda, for the Nvidia nvc (openacc) compiler, that define should not be there, and naturally cl.exe understands none of it all ;-) )

With this, most of the differences in compilation success between "CPU-antlr" and "GPU-antlr" seen in https://new-nightly.mcstas.org/2024-11-18_output/2024-11-18_output.html (col 2-3 vs 4) should go away...

@g5t g5t merged commit 169286b into main Nov 20, 2024
14 checks passed
@g5t g5t deleted the 81-copy-upstream-fixes branch February 2, 2025 12:43
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