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

Add AccessedVariable struct and logic #238

Merged

Conversation

spencer-lunarg
Copy link
Contributor

for both #224 and #226 there is a need to get the underlying mapping between an access (ex OpLoad) and the OpVariable it touched

Currently we just tracked the uint32_t in order to sett the accessed boolean

This creates the ground work the for SpvReflectPrvAccessedVariable to store this logic

} break;
case SpvOpCopyMemory:
case SpvOpCopyMemorySized: {
CHECKED_READU32(p_parser, p_node->word_offset + 2, p_func->accessed_ptrs[p_func->accessed_ptr_count]);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a test for OpCopyMemory.. we were off by 1 as the Target and Source operand are 1 and 2

SpvReflectDescriptorBinding* p_binding = &p_module->descriptor_bindings[i];
for (uint32_t j = 0; j < used_acessed_count; j++) {
if (used_accesses[j].variable_ptr == p_binding->spirv_id) {
p_binding->accessed = 1;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the goal here is we can find all the accesses pointing to this descriptor binding variable

Copy link
Contributor

@chaoticbob chaoticbob left a comment

Choose a reason for hiding this comment

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

Looks good overall. Just some nits with formatting and var names. Not crucial but would be cool to have those changes before merge.

@@ -270,23 +290,24 @@ static bool SearchSortedUint32(const uint32_t* arr, size_t size, uint32_t target
return false;
}

static SpvReflectResult IntersectSortedUint32(const uint32_t* p_arr0, size_t arr0_size, const uint32_t* p_arr1, size_t arr1_size,
uint32_t** pp_res, size_t* res_size) {
static SpvReflectResult IntersectSortedAccessedVariable(const SpvReflectPrvAccessedVariable* p_arr0, size_t arr0_size,
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be one param per line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

spirv_reflect.c Outdated Show resolved Hide resolved
spirv_reflect.c Outdated Show resolved Hide resolved
spirv_reflect.c Outdated Show resolved Hide resolved
@spencer-lunarg spencer-lunarg merged commit 896de4f into KhronosGroup:main Dec 18, 2023
5 checks passed
@spencer-lunarg spencer-lunarg deleted the spencer-variable-access branch December 18, 2023 03:52
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.

None yet

2 participants