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.

Sign up for GitHub

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

Jump to bottom

[SYCL] Rename sycl-fusion to sycl-jit #14762

Merged
merged 16 commits into from
Aug 6, 2024
Merged

[SYCL] Rename sycl-fusion to sycl-jit #14762

merged 16 commits into from
Aug 6, 2024

Conversation

jchlanda
Copy link
Contributor

@jchlanda jchlanda commented Jul 25, 2024 •
edited
Loading

This patch renames the directory (sycl-fusion to sycl-jit) as well
as the involved library names (following the same principle: fusion to
jit).
To keep the user facing names consistent buildbot switches were updated.
And finally the codeowners file had to reflect the directory change.

@jchlanda jchlanda closed this Jul 25, 2024
@jchlanda jchlanda reopened this Jul 25, 2024
@jchlanda jchlanda changed the title WIP: fusion -> jip rename [SYCL] Rename sycl-fusion to sycl-jit Jul 25, 2024
@jchlanda jchlanda marked this pull request as ready for review July 25, 2024 10:27
@jchlanda jchlanda requested review from a team and bader as code owners July 25, 2024 10:27
@jchlanda
Copy link
Contributor Author

This is a follow up from the #14280 where the library/directory rename was discussed.

@jchlanda
[SYCL] Rename sycl-fusion to sycl-jit
544f175 
This patch renames the directory (`sycl-fusion` to `sycl-jit`) as well
as the involved library names (following the same principle: `fusion` to
`jit`).
To keep the user facing names consistent buildbot switches were updated.
And finally the codeowners file had to reflect the directory change.
@jchlanda
Copy link
Contributor Author

Code formatting is failing bad for this patch, but I'm not sure if it should be trusted, seems like it's in python files which have never been formatted in the first place, for instance conifgure.py.

sommerlukas
sommerlukas reviewed Jul 25, 2024
View reviewed changes
Copy link
Contributor

@sommerlukas sommerlukas left a comment

Choose a reason for hiding this comment

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

One minor nit about capitalization.

There's also a few things missing that should be renamed, e.g., the CMake option SYCL_ENABLE_KERNEL_FUSION and related options. Potentially also macros should be renamed, while preserving feature test macros from the extension with the name specified by the extension.

sycl-jit/passes/CMakeLists.txt Outdated
@@ -1,5 +1,5 @@
# Module library for usage as library/pass-plugin with LLVM opt.
add_llvm_library(SYCLKernelFusion MODULE
add_llvm_library(SYCLKernelJit MODULE
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we maintain capitalization of "JIT" here, i.e., SYCLKernelJIT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, will apply the same logic to SYCLKerneJitPasses.

@jchlanda jchlanda requested review from a team as code owners August 2, 2024 06:06
@jchlanda jchlanda removed request for a team August 2, 2024 07:47
@AlexeySachkov AlexeySachkov removed the request for review from a team August 2, 2024 08:19
gmlueck
gmlueck approved these changes Aug 2, 2024
View reviewed changes
Copy link
Contributor

@gmlueck gmlueck left a comment

Choose a reason for hiding this comment

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

Change to Getting Started Guide LGTM.

@sommerlukas sommerlukas closed this Aug 5, 2024
@sommerlukas sommerlukas reopened this Aug 5, 2024
@sommerlukas sommerlukas merged commit 1688e41 into intel:sycl Aug 6, 2024
30 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aelovikov-intel aelovikov-intel aelovikov-intel approved these changes

@hdelan hdelan hdelan approved these changes

@omarahmed1111 omarahmed1111 omarahmed1111 approved these changes

@maarquitos14 maarquitos14 maarquitos14 approved these changes

@AlexeySachkov AlexeySachkov AlexeySachkov approved these changes

@gmlueck gmlueck gmlueck approved these changes

@sommerlukas sommerlukas sommerlukas approved these changes

@bader bader bader approved these changes

@frasercrmck frasercrmck Awaiting requested review from frasercrmck frasercrmck was automatically assigned from intel/llvm-reviewers-cuda

@npmiller npmiller Awaiting requested review from npmiller

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.