-
Notifications
You must be signed in to change notification settings - Fork 151
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
Correctly report liveness of variables passed as function parameters #267
Correctly report liveness of variables passed as function parameters #267
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR, looks good overall. Just a two nits.
const uint32_t result_index = p_node->word_offset + 2; | ||
for (uint32_t j = 0, parameter_count = p_node->word_count - 4; j < parameter_count; j++) { | ||
const uint32_t ptr_index = p_node->word_offset + 4 + j; | ||
SpvReflectPrvAccessedVariable* access_ptr = &p_func->accessed_variables[p_func->accessed_variable_count]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does access_ptr
indicate that it's accessing a pointer or is the _ptr
suffix to indicate that this is variable is a pointer? If it's the latter, then it should be p_access
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line was copied and pasted from the SpvOpLoad..SpvOpImageTexelPointer case below (see line 1204). I can change it on this line, but perhaps it's better to fix both lines at once in a separate PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, apologies I didn't see that. You're right, that should be handled in a separate PR, not critical.
a22e23a
to
16ac489
Compare
@@ -101,7 +101,7 @@ all_descriptor_bindings: | |||
accessed: 1 | |||
uav_counter_id: 4294967295 | |||
uav_counter_binding: | |||
ByteAddressBuffer offsets: [4, 5, 11, 13] | |||
ByteAddressBuffer offsets: [13, 5, 11, 4] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
little concerned why this was flipped here, but not in user_type/byte_address_buffer_3.spv.yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
less concern because the order wasn't important/required
Closes #102