Skip to content

Conversation

s-perron
Copy link
Contributor

@s-perron s-perron commented Jun 18, 2025

The current use of the validation layer will emit a message, but it will
not cause tests to fail. By adding a call back that returns true for
error messages and warnings, test that have a validation error or
warning will fail.

@s-perron
Copy link
Contributor Author

@spall I will eventually want to get this change in. I noticed that some DXC issues showed up, and not all of them are problems in the compiler. I'll get the Vulkan API code clean before I ask to merge this. Failing tests will get marks an XFAIL with a bug opened against them.

Is that reasonable?

@llvm-beanz
Copy link
Collaborator

@s-perron, I was discussing related issues with @spall last night. We definitely want to get this in. Sarah spent time yesterday chasing a failure that had validation errors which didn't cause the test to fail outright, instead it was failing in verification.

Filing a tracking issue on the relevant compiler(s) and XFAILing the tests seems like the right approach to me.

@s-perron s-perron force-pushed the vvl branch 2 times, most recently from c3bac03 to cf9299d Compare June 20, 2025 16:46
@s-perron s-perron marked this pull request as ready for review June 20, 2025 16:47
The current use of the validation layer will emit a message, but it will
not case a test to fail. By adding a call back that returns true for
error messages and warnings, test that have a validation error or
warning will fail.

I've made other changes to keep tests that already pass passing.

xfail test/Basic/TestPipeline.test.

Modify the VkBufferUsageFlagBits for cbuffers. They are uniform buffers
not uniform texel buffers.
@s-perron
Copy link
Contributor Author

FYI: I'm trying to track down why the Intel tests had validation error for f2aadab. I do not have an Intel GPU available right now, so I'll be updating to PR to see what works.

@s-perron s-perron requested a review from spall June 24, 2025 13:58
@s-perron s-perron merged commit 9bb6cd5 into llvm:main Jun 24, 2025
9 checks passed
@spall
Copy link
Collaborator

spall commented Jun 24, 2025

I saw it go purple right as I was about to comment about the assertion failures. So, the assertion is due to support for handlefromimplicitbinding not being added yet to spirv. I can fix these tests so they don't fail by providing a register for the RWBuffer's. I believe the issue you filed in llvm is already being tracked in @hekota 's work.

@s-perron
Copy link
Contributor Author

Sorry, I say a green checkmark from you above, and did not realize that was still the first review from you, and not the second. I was too quick to merge.

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