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.

Sign up for GitHub

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

Jump to bottom

Add support for mutable descriptors #2243

Merged
merged 7 commits into from
Feb 28, 2023
Merged

Add support for mutable descriptors #2243

merged 7 commits into from
Feb 28, 2023

Conversation

ashleysmithamd
Copy link
Contributor

Code to add support for the extension VK_EXT_mutable_descriptor_type. This extension allows a descriptor to be specified as being one of a number of different descriptor types. Since this type can change we need to keep track of the stride for arrays of descriptors.

@ashleysmithamd ashleysmithamd requested a review from a team as a code owner January 26, 2023 10:54
@amdvlk-admin
Copy link

Can one of the admins verify this patch?

@amdrexu amdrexu requested a review from nhaehnle January 27, 2023 15:54
@nhaehnle
Copy link
Member

ok to test

nhaehnle
nhaehnle reviewed Jan 30, 2023
View reviewed changes
include/vkgcDefs.h Outdated Show resolved Hide resolved
lgc/builder/DescBuilder.cpp Outdated Show resolved Hide resolved
lgc/builder/DescBuilder.cpp Outdated Show resolved Hide resolved
lgc/builder/DescBuilder.cpp Outdated Show resolved Hide resolved
llpc/context/llpcPipelineContext.cpp Outdated Show resolved Hide resolved
@amdvlk-admin
Copy link

165d202 Jenkins build error.
/home/amdvlk/jenkins/workspace/vulkan/sanitized-opensource/Github-PR/llpc-github-pr/driver_build/drivers/xgl/icd/api/vk_pipeline_layout.cpp:1074:5: error: braces around scalar initializer for type ‘unsigned int’
};
^
[2964/3012] Building CXX object icd/CMakeFiles/xgl.dir/api/vk_physical_device.cpp.o
[2965/3012] Building CXX object icd/CMakeFiles/xgl.dir/api/vk_image.cpp.o
[2966/3012] Building CXX object icd/CMakeFiles/xgl.dir/api/vk_pipeline_cache.cpp.o

Flakebi
Flakebi reviewed Feb 17, 2023
View reviewed changes
.gitmodules Outdated Show resolved Hide resolved
nhaehnle
nhaehnle reviewed Feb 21, 2023
View reviewed changes
Copy link
Member

@nhaehnle nhaehnle left a comment

Choose a reason for hiding this comment

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

This looks pretty good already, but I still have some comments inline.

lgc/builder/DescBuilder.cpp Outdated Show resolved Hide resolved
lgc/builder/DescBuilder.cpp Outdated Show resolved Hide resolved
lgc/builder/DescBuilder.cpp Outdated Show resolved Hide resolved
@ashleysmithamd
Use PoisonValue instead of UndefValue, remove unused code since we ca…
3df3deb 
…n't get a descriptor in user data, remove redundant if logic, run clang format, fix typo
@ashleysmithamd
Fix comment
9d35033
@amdvlk-admin
Copy link

Test summary for commit 9d35033

CTS tests (Failed: 0/211534)
  • Built with version 1.3.0.0
    • Ubuntu 22.04, Navi3x
    • Passed: 39251/70518 (55.7%)
    • Failed: 0/70518 (0.0%)
    • Not Supported: 31267/70518 (44.3%)
    • Warnings: 0/70518 (0.0%)
    Ubuntu 20.04, Navi2x
    • Passed: 39499/70507 (56.0%)
    • Failed: 0/70507 (0.0%)
    • Not Supported: 31008/70507 (44.0%)
    • Warnings: 0/70507 (0.0%)
    Rhel 9.0, Gfx10
    • Passed: 36346/70509 (51.5%)
    • Failed: 0/70509 (0.0%)
    • Not Supported: 34163/70509 (48.5%)
    • Warnings: 0/70509 (0.0%)

@amdvlk-admin
Copy link

Test summary for commit 4802d89

CTS tests (Failed: 0/211523)
  • Built with version 1.3.0.0
    • Ubuntu 22.04, Navi3x
    • Passed: 39249/70507 (55.7%)
    • Failed: 0/70507 (0.0%)
    • Not Supported: 31258/70507 (44.3%)
    • Warnings: 0/70507 (0.0%)
    Ubuntu 20.04, Navi2x
    • Passed: 39499/70507 (56.0%)
    • Failed: 0/70507 (0.0%)
    • Not Supported: 31008/70507 (44.0%)
    • Warnings: 0/70507 (0.0%)
    Rhel 9.0, Gfx10
    • Passed: 36346/70509 (51.5%)
    • Failed: 0/70509 (0.0%)
    • Not Supported: 34163/70509 (48.5%)
    • Warnings: 0/70509 (0.0%)

nhaehnle
nhaehnle approved these changes Feb 21, 2023
View reviewed changes
Copy link
Member

@nhaehnle nhaehnle left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM now!

@Degerz Degerz mentioned this pull request Feb 25, 2023
Closed
@nhaehnle nhaehnle merged commit 22f16f9 into GPUOpen-Drivers:dev Feb 28, 2023
@ashleysmithamd ashleysmithamd deleted the mutable_descriptor_support branch March 3, 2023 15:22
@ashleysmithamd ashleysmithamd mentioned this pull request Mar 3, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Flakebi Flakebi Flakebi left review comments

@nhaehnle nhaehnle nhaehnle approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ashleysmithamd @amdvlk-admin @nhaehnle @Flakebi