Skip to content

Add DEVICE_PROPERTY support#1034

Merged
dj2 merged 8 commits intogoogle:mainfrom
archimedus:device_property_check
May 13, 2024
Merged

Add DEVICE_PROPERTY support#1034
dj2 merged 8 commits intogoogle:mainfrom
archimedus:device_property_check

Conversation

@archimedus
Copy link
Contributor

For certain cases device properties need to be checked same way as device features.
For example VK_KHR_shader_float_controls extension have a set of fields (DenormPreserve) that are available in VkPhysicalDeviceFloatControlsProperties.
Amber does not allow these to be checked and also does not have appropriate syntax for checking properties.
This pull request adds both.

@google-cla
Copy link

google-cla bot commented Mar 28, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@archimedus
Copy link
Contributor Author

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

I don't get it: I don't have google account. What should I do?

@dj2
Copy link
Collaborator

dj2 commented Mar 28, 2024

I don't believe you need a google account. In order to accept the contribution you'll need to go to https://cla.developers.google.com/ and sign the CLA.

@archimedus
Copy link
Contributor Author

I don't believe you need a google account. In order to accept the contribution you'll need to go to https://cla.developers.google.com/ and sign the CLA.

Thank you for quick reply.

I skim briefly through the page and find nothing more useful.

I scrolled down and click "Manage your agreements" at bottom and ... alas "Please login to google."

Any other thoughts?

@dj2
Copy link
Collaborator

dj2 commented Mar 28, 2024

Yea, turns out I was wrong, from the FAQ on that page A Google account is required to sign the CLA. As far as I can see, there is no way to sign the CLA without one, and without a signed CLA we are unable to accept the PR.

Copy link
Collaborator

@dj2 dj2 left a comment

Choose a reason for hiding this comment

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

Can you please update the docs/amber_script.md to list the new command, and if it makes sense, add a test in tests/cases which exercises the property?

@dj2
Copy link
Collaborator

dj2 commented Apr 9, 2024

The code looks good to me. Once the CLA is sorted out I think this is good to go.

@dj2
Copy link
Collaborator

dj2 commented May 7, 2024

kokoro:run

@archimedus
Copy link
Contributor Author

kokoro:run

@dj2
Copy link
Collaborator

dj2 commented May 7, 2024

Something has gone strange with the bots and they're failing to compile in, it looks like, swiftshader? I'm going to put up a CL and see if I can figure out what's gone wrong.

@dj2
Copy link
Collaborator

dj2 commented May 7, 2024

I'm still working on the bots. Needs to have dependencies updated but getting all of them to the right versions is proving difficult. #1037 is my work in progress CL, but that needs a change to SwiftShader so it can work with a newer SPIRV-Tools.

@dj2
Copy link
Collaborator

dj2 commented May 9, 2024

Ok, if you can rebase this on top of main, I fixed up the various bot issues and got a green build and landed it.

@archimedus archimedus force-pushed the device_property_check branch from bb8a756 to 7f7cab0 Compare May 10, 2024 11:46
@archimedus
Copy link
Contributor Author

kokoro:run

@dj2 dj2 added the kokoro:run label May 13, 2024
@dj2 dj2 merged commit 0f003c2 into google:main May 13, 2024
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