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][RTC] Rework handling of build_options #17405

Merged
merged 6 commits into from
Mar 19, 2025

Conversation

jopperm
Copy link
Contributor

@jopperm jopperm commented Mar 12, 2025

Reject user-supplied arguments a bit more coarsely based on their kind or relation to an unsupported feature.

Signed-off-by: Julian Oppermann <julian.oppermann@codeplay.com>
@jopperm jopperm self-assigned this Mar 12, 2025
@jopperm jopperm requested review from a team as code owners March 12, 2025 12:26
@jopperm jopperm requested a review from uditagarwal97 March 12, 2025 12:26
@jopperm jopperm requested a review from cperkinsintel March 12, 2025 12:27
@jopperm
Copy link
Contributor Author

jopperm commented Mar 12, 2025

I'll document the unsupported options we settle on here in the extension's implementation notes section in a separate PR.

Signed-off-by: Julian Oppermann <julian.oppermann@codeplay.com>
@jopperm
Copy link
Contributor Author

jopperm commented Mar 14, 2025

Companion spec PR: #17459

std::vector<std::string> flags{"-Xs '-doubleGRF'",
"-Xs'-Xfinalizer \"-printregusage\"'"};
std::vector<std::string> flags{"-Xs", "-doubleGRF",
"-XsXfinalizer \"-printregusage\""};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cperkinsintel This occurred to me while working on the documentation:

  • Do we still need to support the implicit unquoting, given build_options are always a vector? I didn't find that in the "normal" option handling (but might have overlooked it).
  • The -Xs option says that for the joined form, a dash is inserted automatically, so -XsDfoo becomes -Dfoo when passed to the target compiler.

jopperm added 2 commits March 14, 2025 06:40
Signed-off-by: Julian Oppermann <julian.oppermann@codeplay.com>
Signed-off-by: Julian Oppermann <julian.oppermann@codeplay.com>
@jopperm
Copy link
Contributor Author

jopperm commented Mar 17, 2025

I realised that invalid options shall trigger an exception with errc::invalid (instead of errc::build) per the spec. Addressing this now.

… detected

Signed-off-by: Julian Oppermann <julian.oppermann@codeplay.com>
@jopperm
Copy link
Contributor Author

jopperm commented Mar 18, 2025

I realised that invalid options shall trigger an exception with errc::invalid (instead of errc::build) per the spec. Addressing this now.

Done. @sommerlukas you may want to review b8b5d86.

@sommerlukas sommerlukas self-requested a review March 18, 2025 08:54
Copy link
Contributor

@KseniyaTikhomirova KseniyaTikhomirova left a comment

Choose a reason for hiding this comment

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

Synced internally about test changes. LGTM.

@sommerlukas sommerlukas merged commit 3a589a1 into intel:sycl Mar 19, 2025
33 of 34 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