Skip to content

[SYCL] Change command line options for Spec Const lowering in sycl-post-link #10741

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 5 commits into from
Aug 11, 2023

Conversation

maksimsab
Copy link
Contributor

@maksimsab maksimsab commented Aug 8, 2023

Replace rt and default values of command line option -spec-const with native and emulation respectively.

@maksimsab maksimsab requested a review from a team as a code owner August 9, 2023 12:31
@maksimsab maksimsab changed the title [SYCL] Refactor SpecConst lowering command line option in sycl-post-link [SYCL] Change command line options for Spec Const lowering in sycl-post-link Aug 9, 2023
Copy link
Contributor

@PietroGhg PietroGhg left a comment

Choose a reason for hiding this comment

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

Native CPU LGTM

Comment on lines 183 to 186
cl::values(clEnumValN(SC_USE_NATIVE, "native",
"lower spec constants to native spirv instructions"),
clEnumValN(SC_USE_EMULATION, "emulation",
"replace spec constants with emulation technique")),
Copy link
Contributor

Choose a reason for hiding this comment

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

i would personally like if we had also left info about rt/c++ default spec constants in description, because now it's mostly describing the implementation, not the meaning. does it make sense?

Copy link
Contributor Author

@maksimsab maksimsab Aug 10, 2023

Choose a reason for hiding this comment

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

Lowering to default C++ values has been dropped here e50ae05#diff-f9a582625f70ea58ef88b7713c591e8187629cca507b424557a3e4664e23848fL147 when we removed support of sycl_ext_oneapi_spec_constants.
The current value name emulation and desc reflects more precisely what it does.
Changed description a bit.

@AlexeySachkov AlexeySachkov merged commit 20d3837 into intel:sycl Aug 11, 2023
@maksimsab maksimsab deleted the refactor_spec_const_param branch August 11, 2023 13:07
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.

6 participants