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] Handle invalid arg for residency code. #3064

Merged
merged 2 commits into from
Aug 20, 2020

Conversation

ehsannas
Copy link
Contributor

@ehsannas ehsannas commented Aug 4, 2020

Fixes #3045

@ehsannas ehsannas added the spirv Work related to SPIR-V label Aug 4, 2020
@ehsannas ehsannas requested a review from jaebaek August 4, 2020 17:42
@ehsannas ehsannas self-assigned this Aug 4, 2020
@AppVeyorBot
Copy link

@@ -3216,6 +3216,13 @@ SpirvInstruction *SpirvEmitter::processBufferTextureLoad(
return nullptr;
}

if (residencyCode && residencyCode->isRValue()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What happens if residencyCode is nullptr?
Why don't we change this condition to if (residencyCode == nullptr || residencyCode->isRValue()) {?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this function (SpirvEmitter::processBufferTextureLoad) handles the Load methods. There are 2 Load methods that are defined for textures and buffer: one method that just does the load, and one method that does the load and also returns the status of the operation.

For example, for Texture2D we have these 2 methods: (spec)

Load(int,int) : Reads texture data.
Load(int,int,uint) : Reads texture data and returns status of the operation.

In the first method, residencyCode is in fact nullptr and it should not be an error.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Got it. It would be better to add a comment to briefly explain it.

@AppVeyorBot
Copy link

@ehsannas ehsannas merged commit 0e04763 into microsoft:master Aug 20, 2020
@ehsannas ehsannas deleted the fix-3045 branch August 20, 2020 18:27
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.

[SPIRV] Invalid shader crashes compiler
3 participants