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(sycl): Replaces pragma once with include guards #1709

Merged
merged 1 commit into from
Oct 27, 2024
Merged

Conversation

jrwrigh
Copy link
Collaborator

@jrwrigh jrwrigh commented Oct 27, 2024

OpenCL doesn't really like pragma once evidently. I think we've been 'getting away' with it previously as the JIT processing automatically doesn't allow for nested includes, but the same is not done for <ceed/types.h>?

@jeremylt
Copy link
Member

I'd double check the other Sycl JiT files

Note - I'm out next week and won't be approving anything

OpenCL doesn't really like pragma once evidently. I think we've been
'getting away' with it previously as the JIT processing automatically
doesn't allow for nested includes, but the same is not done for
<ceed/types.h>?
@jedbrown
Copy link
Member

Rebased after merging #1703

@jedbrown jedbrown merged commit 6441472 into main Oct 27, 2024
28 checks passed
@jedbrown jedbrown deleted the jrwrigh/fix_sycl branch October 27, 2024 14:47
@@ -7,7 +7,8 @@

/// @file
/// Public header for types and macros used in user QFunction source code
#pragma once
#ifndef CEED_QFUNCTION_DEFS_H
Copy link
Member

Choose a reason for hiding this comment

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

Small note - I've seen some people recommend both. Not sure I fully understand the argument why though

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, both is a bit perplexing... Maybe to avoid possible guard name conflicts?

Copy link
Member

Choose a reason for hiding this comment

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

I think for a while #pragma once gave faster compile times, but nowadays that's less of a factor

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.

3 participants