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

[SYCL] Rename DoubleGRF to LargeGRF #7284

Merged
merged 1 commit into from
Nov 8, 2022
Merged

[SYCL] Rename DoubleGRF to LargeGRF #7284

merged 1 commit into from
Nov 8, 2022

Conversation

sarnex
Copy link
Contributor

@sarnex sarnex commented Nov 4, 2022

This change renames double GRF to large GRF both for users and internally in the compiler. We're doing this because we got direct feedback from customer facing engineers that we should use the large GRF terminology, and it also makes the naming consistent with other compiler work we are doing.

For the user,

set_kernel_properties(kernel_properties::use_double_grf);

still works, it will just throw a deprecated warning and will be removed in a future release.

The new way is

set_kernel_properties(kernel_properties::use_large_grf);

There should be no ABI break because we still check the previous image property name in the program manager, so applications built with an old compiler work using the runtime from a new compiler. I confirmed this with manual testing.

I will update the system test here to test the new flag as well: https://github.com/intel/llvm-test-suite/blob/intel/SYCL/DeviceCodeSplit/double-grf.cpp

Signed-off-by: Sarnie, Nick nick.sarnie@intel.com

Signed-off-by: Sarnie, Nick <nick.sarnie@intel.com>
@sarnex sarnex marked this pull request as ready for review November 4, 2022 14:50
@sarnex sarnex requested review from a team as code owners November 4, 2022 14:50
@sarnex sarnex requested a review from steffenlarsen November 4, 2022 14:50
@sarnex
Copy link
Contributor Author

sarnex commented Nov 4, 2022

@kbobrovs @asudarsa Hey guys, mind taking a look at this one? Thanks

@asudarsa
Copy link
Contributor

asudarsa commented Nov 4, 2022

Will check before EOD. Thanks

Copy link
Contributor

@steffenlarsen steffenlarsen left a comment

Choose a reason for hiding this comment

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

Runtime changes LGTM.

Copy link
Contributor

@asudarsa asudarsa left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks

@kbobrovs kbobrovs merged commit ab2a42c into intel:sycl Nov 8, 2022
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.

5 participants