Skip to content

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
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
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