Skip to content

[SYCL][E2E] Fix LIT feature name - #23088

Open
uditagarwal97 wants to merge 1 commit into
syclfrom
uditagarwal97-patch-1
Open

[SYCL][E2E] Fix LIT feature name#23088
uditagarwal97 wants to merge 1 commit into
syclfrom
uditagarwal97-patch-1

Conversation

@uditagarwal97

Copy link
Copy Markdown
Contributor

This test always gets skipped due to incorrect LIT feature name.

@uditagarwal97 uditagarwal97 self-assigned this Sep 2, 2026
@@ -1,4 +1,4 @@
// REQUIRES: gpu, level-zero

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I suggest removing -/_ from the lit feature name to avoid similar bugs in the future. It won't prevent other typos, but this specific one is just ridiculous.

@uditagarwal97 uditagarwal97 Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We have a lot of LIT features with - or _ in their name, renaming all of them would be a big change. How about we instead make LIT throw an error if it encounters an unknown LIT feature?
IIRC we already maintain a list of approved LIT features to decide if it's build-time or runtime feature.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We have a lot of LIT features with - or _ in their name, renaming all of them would be a big change. How about we instead make LIT throw an error if it encounters an unknown LIT feature? IIRC we already maintain a list of approved LIT features to decide if it's build-time or runtime feature.

That's a good idea, I believe we have more typos. I tried to implement it previously, but it got bogged down in discussion.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@KornevNikita I like the solution you proposed in #16019 - it's easy to maintain, extend, and require minimal changes. Let me try it out

@uditagarwal97
uditagarwal97 marked this pull request as ready for review September 3, 2026 14:10
@uditagarwal97
uditagarwal97 requested a review from a team as a code owner September 3, 2026 14:10
@uditagarwal97

Copy link
Copy Markdown
Contributor Author

@intel/unified-runtime-reviewers-level-zero ping

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