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

Update VK_AMDX_shader_enqueue to v2 #2442

Merged
merged 2 commits into from
Oct 10, 2024

Conversation

Tobski
Copy link
Contributor

@Tobski Tobski commented Oct 4, 2024

  • Adds mesh shader node support
  • Adds two new limits to workgroups
  • Variable scratch size support

 - Adds mesh shader node support
 - Adds two new limits to workgroups
 - Variable scratch size support
@Tobski
Copy link
Contributor Author

Tobski commented Oct 4, 2024

From a quick look, it looks like the ash build is failing because it has old definitions for commands that were updated in this extension already in the crate, but would be good if we can find someone to check that...

@oddhack
Copy link
Contributor

oddhack commented Oct 5, 2024

From a quick look, it looks like the ash build is failing because it has old definitions for commands that were updated in this extension already in the crate, but would be good if we can find someone to check that...

Pinging @MarijnS95

@MarijnS95
Copy link
Contributor

It looks like our hand-written "higher level" command wrappers for this extension are incompatible with the new signature, and would have to be updated via releasing the changes d regenerating + updating the ash crate.

However, I couldn't quickly find the right documentation on this but at least https://docs.vulkan.org/spec/latest/chapters/extensions.html#VkExtensionProperties says for the specVersion field (that's bumped from 1 to 2 here):

specVersion is the version of this extension. It is an integer, incremented with backward compatible changes.

Is changing the function signature considered "backwards compatible"?

@oddhack
Copy link
Contributor

oddhack commented Oct 5, 2024

Is changing the function signature considered "backwards compatible"?

No. But this is an old vendor "experimental" extension (AMDX vendor ID), which is something like a "beta" extension is in current practice. That means that the API is not necessarily going to be stable.

@Tobski
Copy link
Contributor Author

Tobski commented Oct 7, 2024

Is changing the function signature considered "backwards compatible"?

No. But this is an old vendor "experimental" extension (AMDX vendor ID), which is something like a "beta" extension is in current practice. That means that the API is not necessarily going to be stable.

Yea it's not meant to be stable, that's why it has the 'provisional="true"' attribute in the xml, as well as the AMDX tag. New revisions can just as well be entirely new extensions for provisional extensions. They're not generally available in consumer drivers, and are provided for developer feedback rather than productization.

Copy link
Contributor

@oddhack oddhack left a comment

Choose a reason for hiding this comment

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

LGTM. Please tag Signed Off to Merge when you're ready (ideally today).

(Edit: and resolve conflicts, of course).

# Conflicts:
#	chapters/executiongraphs.adoc
@Tobski
Copy link
Contributor Author

Tobski commented Oct 9, 2024

LGTM. Please tag Signed Off to Merge when you're ready (ideally today).

(Edit: and resolve conflicts, of course).

Done. Still failing Ash crate CI but @MarijnS95's PR should fix that once accepted.

@oddhack oddhack merged commit 03f2654 into KhronosGroup:main Oct 10, 2024
11 of 12 checks passed
@MarijnS95
Copy link
Contributor

Thanks! We're doing the update in ash-rs/ash#951, and attempting to prevent issues stemming from allowed breaking changes to provisional extensions in ash-rs/ash#952.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants