Skip to content

Enable most of the cbuffer tests for clang #280

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 2 commits into from
Aug 8, 2025

Conversation

bogner
Copy link
Collaborator

@bogner bogner commented Jul 8, 2025

These have been working for some time but we never enabled the tests.

@bogner
Copy link
Collaborator Author

bogner commented Jul 8, 2025

Test failure is the XPASS being resolved in #284

Copy link
Collaborator

@llvm-beanz llvm-beanz left a comment

Choose a reason for hiding this comment

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

General feedback: should these be unsupported or XFAIL so that we know when they start working?

# https://github.com/llvm/llvm-project/issues/110722
# XFAIL: Clang
# https://github.com/llvm/llvm-project/issues/138996
# UNSUPPORTED: Clang
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want this UNSUPPORTED or XFAIL?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

XFAIL. I'll update these.

@@ -1,4 +1,4 @@
if 'Clang' in config.available_features:
if 'Clang-Vulkan' in config.available_features:
config.unsupported = True
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we just be enabling these now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, none of these tests pass under vulkan. I could remove this and add XFAILs to the tests directly if you think that makes more sense.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't have a strong feeling, I just want to make sure that we don't somehow lose track and not enable the tests once they do start passing.

bogner added 2 commits August 7, 2025 09:28
These have been working for some time but we never enabled the tests.
@bogner bogner force-pushed the 2025-07-08-cbuffer-tests branch from 2bba62c to c8007c1 Compare August 7, 2025 16:55
@bogner
Copy link
Collaborator Author

bogner commented Aug 8, 2025

pre-checkin CI is in a bit of a rough spot, but the cbuffer tests themselves are passing.

@bogner bogner merged commit 24b2efb into llvm:main Aug 8, 2025
6 of 9 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