Skip to content

[SYCL][Driver][NFC] Reorganize SYCL based options #14979

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 2 commits into from
Aug 9, 2024

Conversation

mdtoguchi
Copy link
Contributor

The SYCL based options in the Options.td table were a bit scattered about. Reorganize the options to be in a more centralized area, allowing for a more general adaptation of using sycl_Group. Items within sycl_Group will automatically be allowed for Windows and Linux command lines.

Some test cleanup involving some expected warnings have also been addressed, either due by proper enabling for Windows or improper warning expectations.

The SYCL based options in the Options.td table were a bit scattered
about.  Reorganize the options to be in a more centralized area,
allowing for a more general adaptation of using sycl_Group.  Items
within sycl_Group will automatically be allowed for Windows and Linux
command lines.

Some test cleanup involving some expected warnings have also been
addressed, either due by proper enabling for Windows or improper warning
expectations.
@mdtoguchi mdtoguchi marked this pull request as ready for review August 7, 2024 15:23
@mdtoguchi mdtoguchi requested a review from a team as a code owner August 7, 2024 15:23
@@ -187,7 +187,8 @@ def opencl_Group : OptionGroup<"<opencl group>">, Group<f_Group>,
DocName<"OpenCL options">;

def sycl_Group : OptionGroup<"<SYCL group>">, Group<f_Group>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is Group<f_Group> needed here?
Will DocName<"SYCL options"> override the DocName defined in Group<f_Group>

def f_Group : OptionGroup<"<f group>">, Group<CompileOnly_Group>,
              DocName<"Target-independent compilation options">;

Also, f_Group has CompileOnly_Group, but some of the options in sycl_Group are link time options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The use of f_Group here is more in line with existing usage for offloading specific options such as cuda_Group and offload_Group. Not all options used are specific to the compilation phase, but are part of the overall compilation to create the device binary.

I checked the generated documentation files, and sycl_Group does override f_Group in regards to the DocName

@mdtoguchi
Copy link
Contributor Author

@intel/llvm-gatekeepers, this looks ready for merge, please take a look - thanks!

@sommerlukas sommerlukas merged commit de0260f into intel:sycl Aug 9, 2024
13 checks passed
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.

3 participants