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

Add HIPSYCL_SYCLCC_EXTRA_COMPILE_OPTIONS #824

Conversation

al42and
Copy link
Contributor

@al42and al42and commented Sep 6, 2022

Similar to HIPSYCL_SYCLCC_EXTRA_ARGS, but will only be applied to files explicitly listed in SOURCES argument of add_sycl_to_target.

Examples of flags where it can be useful are various -Wno-... and -ffast-math (which can break some code on host, but can be desired for device kernels).

I'm open to changing the name of the variable.

Testing: https://gist.github.com/al42and/2c21e842702f45b9be44d793a77a7d69, there mkdir build && cmake -DHIPSYCL_TARGETS=omp -Bbuild/ && cmake --build build/

Similar to `HIPSYCL_SYCLCC_EXTRA_ARGS`, but will only be applied to files
explicitly listed in `SOURCES` argument of `add_sycl_to_target`.
@illuhad
Copy link
Collaborator

illuhad commented Sep 28, 2022

This is good in principle, but we need to come up with another name. It's not really device-only, as it also affects host compilation. Maybe a name to focus on the fact that it affects the SYCL compiler, not linker?

@al42and
Copy link
Contributor Author

al42and commented Sep 28, 2022

It's setting COMPILE_OPTIONS, so how about HIPSYCL_SYCLCC_EXTRA_COMPILE_OPTIONS?

@illuhad
Copy link
Collaborator

illuhad commented Sep 28, 2022

I like it!

@al42and al42and changed the title Add HIPSYCL_SYCLCC_EXTRA_ARGS_DEVICE_ONLY Add HIPSYCL_SYCLCC_EXTRA_COMPILE_OPTIONS Sep 29, 2022
@illuhad illuhad merged commit 9e78270 into AdaptiveCpp:develop Sep 30, 2022
@al42and al42and deleted the aa-HIPSYCL_SYCLCC_EXTRA_ARGS_DEVICE_ONLY-flag branch September 30, 2022 22:25
acmnpv pushed a commit to gromacs/gromacs that referenced this pull request Oct 10, 2022
Requires AdaptiveCpp/AdaptiveCpp#824.

Inlining does not affect AMD, but helps tremendously for CUDA.

Refs #4588
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.

None yet

2 participants