Skip to content

Convert sampled consumers to being Instructions instead of IDs#2464

Merged
zoddicus merged 3 commits intoKhronosGroup:masterfrom
zoddicus:issue2463
Mar 19, 2019
Merged

Convert sampled consumers to being Instructions instead of IDs#2464
zoddicus merged 3 commits intoKhronosGroup:masterfrom
zoddicus:issue2463

Conversation

@zoddicus
Copy link
Copy Markdown
Contributor

Changing the stored value for a sampled image consumer to be the
instruction instead of result ID, since not all instructions have
result IDs. Using result IDs led to a potential crash when using
OpReturnValue, which doesn't have a result ID. OpReturnValue is not a
legal consumer, but the validator needs to look at the instruction to
determine this, thus storing the pointer to the instruction, instead
of trying to fetch the pointer using the instruction.

Issue #1528 covers fixing the check.

Fixes #2463

Changing the stored value for a sampled image consumer to be the
instruction instead of result ID, since not all instructions have
result IDs. Using result IDs led to a potential crash when using
OpReturnValue, which doesn't have a result ID. OpReturnValue is not a
legal consumer, but the validator needs to look at the instruction to
determine this, thus storing the pointer to the instruction, instead
of trying to fetch the pointer using the instruction.

Issue #1528 covers fixing the check.

Fixes #2463
Copy link
Copy Markdown
Contributor

@alan-baker alan-baker left a comment

Choose a reason for hiding this comment

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

Thanks for this. Can you add a test for this that validates till the other issue is resolved so we catch any regressions here.

@zoddicus
Copy link
Copy Markdown
Contributor Author

Thanks for this. Can you add a test for this that validates till the other issue is resolved so we catch any regressions here.

Done, created a reduced case of the example provided in the bug.

@zoddicus zoddicus requested a review from alan-baker March 18, 2019 20:18
@zoddicus zoddicus merged commit 08b54d9 into KhronosGroup:master Mar 19, 2019
@zoddicus zoddicus deleted the issue2463 branch March 19, 2019 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[spiv-val] Miscompiled SPIR-V is crashing the validator

2 participants