Skip to content

Conversation

@jrwrigh
Copy link
Collaborator

@jrwrigh jrwrigh commented Jun 20, 2025

Update to include CEED_RUNNING_JIT_PASS to the SYCL backend's definitions. This was added in #1696, but not to the SYCL backend.

Confirmed this works on Aurora.

@jrwrigh
Copy link
Collaborator Author

jrwrigh commented Jun 20, 2025

@kris-rowe @uumesh Any foreseen issues with this?

@kris-rowe
Copy link
Collaborator

Not that I can think of. Does the kernel generation in the SYCL backend need to be updated to use include flags?

@uumesh
Copy link
Contributor

uumesh commented Jun 20, 2025

Is this addition just for consistency? I don't think it will have any effect unless the JIT source is also modified.

@jrwrigh
Copy link
Collaborator Author

jrwrigh commented Jun 20, 2025

Is this addition just for consistency? I don't think it will have any effect unless the JIT source is also modified.

For consistency, and also to allow including character arrays in the header files (see MR here). The latter is more for coding convenience than anything else.

Edit: We do have some HONEE jit sources that use CEED_RUNNING_JIT_PASS to ignore <math.h> and <stdbool.h> include directives, but evidently it's not necessary for the SYCL backend. See

#ifndef CEED_RUNNING_JIT_PASS
#include <math.h>
#include <stdbool.h>
#endif

@jrwrigh
Copy link
Collaborator Author

jrwrigh commented Jun 20, 2025

Does the kernel generation in the SYCL backend need to be updated to use include flags?

It'd probably be nice in the future, but it's not blocking anything in HONEE.

@uumesh
Copy link
Contributor

uumesh commented Jun 20, 2025

That should be fine. I also see that the flag adds a guard to exclude std/math libraries from the QFunctions. I am not sure if that will affect the compilation of OpenCL kernels. For example in the advection functions here
If you can check that this test passes, the rest should be fine as well.

@jrwrigh
Copy link
Collaborator Author

jrwrigh commented Jun 20, 2025

It's been passing tests in HONEE, I'll double check with the libCEED tests real quick though. (HONEE should be a super set of the libCEED fluids tests).

Edit: All the fluids tests pass with the exception of a few segfaults, but those were present before as well.

@jrwrigh jrwrigh requested a review from jeremylt June 20, 2025 18:22
@jeremylt
Copy link
Member

jeremylt commented Jun 23, 2025

This may be useful to add as well:

Lines 87-113

@jrwrigh jrwrigh merged commit 597fda5 into main Jun 23, 2025
29 checks passed
@jrwrigh jrwrigh deleted the jrwrigh/sycl_ceed_jit_define branch June 23, 2025 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants