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

[spirv] Remove BufferBlock for array types #3039

Merged
merged 3 commits into from
Jul 15, 2020

Conversation

jaebaek
Copy link
Collaborator

@jaebaek jaebaek commented Jul 15, 2020

Removing BufferBlock and replacing Uniform to StorageBuffer
must be done for not only the struct type whose interface type
is StorageBuffer but also for an array or runtime array type whose
element type is such struct type.

Fixes #3035

@jaebaek jaebaek requested a review from ehsannas July 15, 2020 03:02
@jaebaek jaebaek self-assigned this Jul 15, 2020
@jaebaek jaebaek added the spirv Work related to SPIR-V label Jul 15, 2020
@AppVeyorBot
Copy link

@Jasper-Bekkers
Copy link

Just tested, this fixes the issue for us.

Copy link
Contributor

@ehsannas ehsannas left a comment

Choose a reason for hiding this comment

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

Just a couple of nits. Thanks

@@ -33,6 +33,23 @@ bool RemoveBufferBlockVisitor::visit(SpirvModule *mod, Phase phase) {
return true;
}

bool RemoveBufferBlockVisitor::HasStorageBufferInterfaceType(
Copy link
Contributor

Choose a reason for hiding this comment

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

s/HasStorageBufferInterfaceType/hasStorageBufferInterfaceType

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

ptrResultType->getStorageClass() !=
spv::StorageClass::StorageBuffer) {
if (ptrResultType->getStorageClass() !=
spv::StorageClass::StorageBuffer) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can now do

if (hasStorageBufferInterfaceType(ptrResultType->getPointeeType()) &&
    ptrResultType->getStorageClass() != spv::StorageClass::StorageBuffer) {
...
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Copy link
Collaborator Author

@jaebaek jaebaek left a comment

Choose a reason for hiding this comment

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

Thank you for your code review. PTAL!

@@ -33,6 +33,23 @@ bool RemoveBufferBlockVisitor::visit(SpirvModule *mod, Phase phase) {
return true;
}

bool RemoveBufferBlockVisitor::HasStorageBufferInterfaceType(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

ptrResultType->getStorageClass() !=
spv::StorageClass::StorageBuffer) {
if (ptrResultType->getStorageClass() !=
spv::StorageClass::StorageBuffer) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@ehsannas ehsannas left a comment

Choose a reason for hiding this comment

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

Thanks!

@AppVeyorBot
Copy link

@ehsannas ehsannas merged commit 1ca25d5 into microsoft:master Jul 15, 2020
@jaebaek jaebaek deleted the remove_bufferblock branch July 15, 2020 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spirv Work related to SPIR-V
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SPIR-V] The result pointer storage class and base pointer storage class in OpAccessChain do not match
4 participants