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

Use of uchar4 generates second copy of OpTypeInt 32 0 #15

Closed
dneto0 opened this issue Jul 14, 2017 · 0 comments
Closed

Use of uchar4 generates second copy of OpTypeInt 32 0 #15

dneto0 opened this issue Jul 14, 2017 · 0 comments
Assignees
Labels
Projects

Comments

@dneto0
Copy link
Collaborator

dneto0 commented Jul 14, 2017

That's invalid by the SPIR-V spec. The validator briefly had that rule turned on, but some bad Vulkan CTS tests have duplicate scalar type decls that are hard to rewrite, so we turned that rule off.

Example input:

kernel void foo(global uchar4* A) {
  *A = (uchar4)(1,2,3,4);
}

Appears to be caused by SPIRVProducerPass::GenerateSPIRVTypes where it says "i8 is added to TypeMap as i32."
The issue is that the Types collection (returned by getTypeList) has entries for i8 and i32. But the i8 will be converted at the last second to an i32. So we end up with duplicate OpTypeInt 32 0.

Here's a fragment of the generated assembly:

%1 =    OpTypeInt 32 0    ; the first one
%2 =    OpTypePointer StorageBuffer %1
%3 =    OpTypeRuntimeArray %1
%4 =    OpTypeStruct %3
%5 =    OpTypePointer StorageBuffer %4
%6 =    OpTypeInt 32 0 ;   oops
%7 =    OpTypeVoid
%8 =    OpTypeFunction %7
%9 =    OpTypeVector %6 3
@dneto0 dneto0 added the bug label Aug 25, 2017
@dneto0 dneto0 added this to Ready to start in General Aug 25, 2017
@dneto0 dneto0 moved this from Ready to start to In progress in General Aug 26, 2017
@dneto0 dneto0 self-assigned this Aug 26, 2017
dneto0 added a commit to dneto0/clspv that referenced this issue Aug 26, 2017
This is incrementally better than what we have now, but I suspect
we should have remapped the types earlier in the flow, and we should
never see the i8 at this point.

Fixes google#15
@dneto0 dneto0 moved this from In progress to Done in General Aug 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
General
  
Done
Development

No branches or pull requests

1 participant