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

Fix issue #761 stores involving GEP of aggregates of pointers hits in… #764

Closed

Conversation

thewilsonator
Copy link

…valid BitCast assert

Handle pointer scalar elements in lib/ReplacePointerBitcastsPass.cpp:BuildFromElements so that we don't generate an invalid bitcast from float addrspace(1)* to i32 as the else branch for regular scalar conversions conversion generates bitcasts to integer types.

@alan-baker
Copy link
Collaborator

Your test doesn't indicate what the expected code is. Bitcasting from a pointer to an integer is not generally allowed in shader SPIR-V and that appears to be what is happening.

When I checkout this branch I still get assertions that the replace all uses with is being used incorrectly.

@thewilsonator
Copy link
Author

Bitcasting from a pointer to an integer is not generally allowed in shader SPIR-V and that appears to be what is happening.

Correct, but this is not present in the source input. The bad cast is generated by the ReplacePointerBitcastsPass because it (currently) doesn't handle pointers in aggregates properly and generates pointer-to-integer bit casts for (aggregate of pointer)-to-pointer bit casts (or the other way around, I don't remember off the top of my head). It should be noted that it does handle pointer-to-pointer bit casts outside of aggregates properly.

Your test doesn't indicate what the expected code is.
When I checkout this branch I still get assertions that the replace all uses with is being used incorrectly.

The test is a simple saxpy kernel, but the intent of the test is to validate that the compiler does not hit assert generating the code for it. Now that CI is enabled I will look into this more, because "It works on my machine".

Thanks!

@thewilsonator
Copy link
Author

"It works on my machine".

Seems I accidentally committed the 64-bit test case file, which won't work because compute SPIR-V is effectively 32 bit (the width of gl_InvocationID. Fixed the style and unused variable warnings.

…its invalid BitCast assert

Handle pointer scalar elements in `lib/ReplacePointerBitcastsPass.cpp:BuildFromElements` so that we don't generate an invalid bitcast from `float addrspace(1)*` to `i32` as  the `else` branch for regular scalar conversions conversion generates bitcasts to integer types.
@thewilsonator
Copy link
Author

I've managed to work around this by not generating those bitcasts in the first place.

@thewilsonator thewilsonator deleted the fix-pointer-bitcasts branch September 20, 2021 01:44
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