Skip to content

Commit

Permalink
Correctly report liveness of variables passed as function parameters (#…
Browse files Browse the repository at this point in the history
…267)

* 関数の引数に渡された変数のアクセス状況を解析するように修正

* Update golden YAML

* Add function parameter access test

---------

Co-authored-by: Awata Yuichi <[email protected]>
  • Loading branch information
daniel-story and Awata Yuichi authored Jul 12, 2024
1 parent b4dc70d commit f368a83
Show file tree
Hide file tree
Showing 8 changed files with 464 additions and 10 deletions.
95 changes: 88 additions & 7 deletions spirv_reflect.c
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,14 @@ typedef struct SpvReflectPrvAccessedVariable {
SpvReflectPrvNode* p_node;
uint32_t result_id;
uint32_t variable_ptr;
uint32_t function_id;
uint32_t function_parameter_index;
} SpvReflectPrvAccessedVariable;

typedef struct SpvReflectPrvFunction {
uint32_t id;
uint32_t parameter_count;
uint32_t* parameters;
uint32_t callee_count;
uint32_t* callees;
struct SpvReflectPrvFunction** callee_ptrs;
Expand Down Expand Up @@ -642,6 +646,7 @@ static void DestroyParser(SpvReflectPrvParser* p_parser) {

// Free functions
for (size_t i = 0; i < p_parser->function_count; ++i) {
SafeFree(p_parser->functions[i].parameters);
SafeFree(p_parser->functions[i].callees);
SafeFree(p_parser->functions[i].callee_ptrs);
SafeFree(p_parser->functions[i].accessed_variables);
Expand Down Expand Up @@ -1095,6 +1100,7 @@ static SpvReflectResult ParseFunction(SpvReflectPrvParser* p_parser, SpvReflectP
size_t first_label_index) {
p_func->id = p_func_node->result_id;

p_func->parameter_count = 0;
p_func->callee_count = 0;
p_func->accessed_variable_count = 0;

Expand All @@ -1105,7 +1111,11 @@ static SpvReflectResult ParseFunction(SpvReflectPrvParser* p_parser, SpvReflectP
break;
}
switch (p_node->op) {
case SpvOpFunctionParameter: {
++(p_func->parameter_count);
} break;
case SpvOpFunctionCall: {
p_func->accessed_variable_count += p_node->word_count - 4;
++(p_func->callee_count);
} break;
case SpvOpLoad:
Expand All @@ -1128,6 +1138,13 @@ static SpvReflectResult ParseFunction(SpvReflectPrvParser* p_parser, SpvReflectP
}
}

if (p_func->parameter_count > 0) {
p_func->parameters = (uint32_t*)calloc(p_func->parameter_count, sizeof(*(p_func->parameters)));
if (IsNull(p_func->parameters)) {
return SPV_REFLECT_RESULT_ERROR_ALLOC_FAILED;
}
}

if (p_func->callee_count > 0) {
p_func->callees = (uint32_t*)calloc(p_func->callee_count, sizeof(*(p_func->callees)));
if (IsNull(p_func->callees)) {
Expand All @@ -1143,6 +1160,7 @@ static SpvReflectResult ParseFunction(SpvReflectPrvParser* p_parser, SpvReflectP
}
}

p_func->parameter_count = 0;
p_func->callee_count = 0;
p_func->accessed_variable_count = 0;
// Now have allocation, fill in values
Expand All @@ -1152,8 +1170,25 @@ static SpvReflectResult ParseFunction(SpvReflectPrvParser* p_parser, SpvReflectP
break;
}
switch (p_node->op) {
case SpvOpFunctionParameter: {
CHECKED_READU32(p_parser, p_node->word_offset + 2, p_func->parameters[p_func->parameter_count]);
(++p_func->parameter_count);
} break;
case SpvOpFunctionCall: {
CHECKED_READU32(p_parser, p_node->word_offset + 3, p_func->callees[p_func->callee_count]);
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];

access_ptr->p_node = p_node;
// Need to track Result ID as not sure there has been any memory access through here yet
CHECKED_READU32(p_parser, result_index, access_ptr->result_id);
CHECKED_READU32(p_parser, ptr_index, access_ptr->variable_ptr);
access_ptr->function_id = p_func->callees[p_func->callee_count];
access_ptr->function_parameter_index = j;
(++p_func->accessed_variable_count);
}
(++p_func->callee_count);
} break;
case SpvOpLoad:
Expand Down Expand Up @@ -1238,14 +1273,12 @@ static SpvReflectResult ParseFunctions(SpvReflectPrvParser* p_parser) {

// Skip over function declarations that aren't definitions
bool func_definition = false;
// Intentionally reuse i to avoid iterating over these nodes more than
// once
for (; i < p_parser->node_count; ++i) {
if (p_parser->nodes[i].op == SpvOpLabel) {
for (size_t j = i; j < p_parser->node_count; ++j) {
if (p_parser->nodes[j].op == SpvOpLabel) {
func_definition = true;
break;
}
if (p_parser->nodes[i].op == SpvOpFunctionEnd) {
if (p_parser->nodes[j].op == SpvOpFunctionEnd) {
break;
}
}
Expand Down Expand Up @@ -3439,6 +3472,44 @@ static SpvReflectResult ParseByteAddressBuffer(SpvReflectPrvParser* p_parser, Sp
return SPV_REFLECT_RESULT_SUCCESS;
}

static SpvReflectResult ParseFunctionParameterAccess(SpvReflectPrvParser* p_parser, uint32_t callee_function_id,
uint32_t function_parameter_index, uint32_t* p_accessed) {
SpvReflectPrvFunction* p_func = NULL;
for (size_t i = 0; i < p_parser->function_count; ++i) {
if (p_parser->functions[i].id == callee_function_id) {
p_func = &(p_parser->functions[i]);
break;
}
}
if (p_func == NULL) {
return SPV_REFLECT_RESULT_ERROR_SPIRV_INVALID_ID_REFERENCE;
}

assert(function_parameter_index < p_func->parameter_count);

for (size_t i = 0; i < p_func->accessed_variable_count; ++i) {
if (p_func->parameters[function_parameter_index] == p_func->accessed_variables[i].variable_ptr) {
SpvReflectPrvAccessedVariable* p_var = &p_func->accessed_variables[i];
if (p_var->function_id > 0) {
SpvReflectResult result =
ParseFunctionParameterAccess(p_parser, p_var->function_id, p_var->function_parameter_index, p_accessed);
if (result != SPV_REFLECT_RESULT_SUCCESS) {
return result;
}
} else {
*p_accessed = 1;
}
// Early out as soon as p_accessed is true
if (*p_accessed) {
return SPV_REFLECT_RESULT_SUCCESS;
}
}
}

*p_accessed = 0;
return SPV_REFLECT_RESULT_SUCCESS;
}

static SpvReflectResult ParseStaticallyUsedResources(SpvReflectPrvParser* p_parser, SpvReflectShaderModule* p_module,
SpvReflectEntryPoint* p_entry, size_t uniform_count, uint32_t* uniforms,
size_t push_constant_count, uint32_t* push_constants) {
Expand Down Expand Up @@ -3535,8 +3606,18 @@ static SpvReflectResult ParseStaticallyUsedResources(SpvReflectPrvParser* p_pars
uint32_t byte_address_buffer_offset_count = 0;

for (uint32_t j = 0; j < used_acessed_count; j++) {
if (p_used_accesses[j].variable_ptr == p_binding->spirv_id) {
p_binding->accessed = 1;
SpvReflectPrvAccessedVariable* p_var = &p_used_accesses[j];
if (p_var->variable_ptr == p_binding->spirv_id) {
if (p_var->function_id > 0) {
result =
ParseFunctionParameterAccess(p_parser, p_var->function_id, p_var->function_parameter_index, &p_binding->accessed);
if (result != SPV_REFLECT_RESULT_SUCCESS) {
SafeFree(p_used_accesses);
return result;
}
} else {
p_binding->accessed = 1;
}

if (HasByteAddressBufferOffset(p_used_accesses[j].p_node, p_binding)) {
byte_address_buffer_offset_count++;
Expand Down
51 changes: 51 additions & 0 deletions tests/issues/102/function_parameter_access.glsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
glslangValidator -V -S frag -g0 --ku -o function_parameter_access.spv function_parameter_access.glsl
*/

#version 450

layout(binding = 0) uniform sampler Sampler;
layout(binding = 1) uniform texture2D Texture;
layout(binding = 2) uniform sampler2D Sampler2D;

layout(binding = 3) uniform sampler NeverAccessedSampler;
layout(binding = 4) uniform texture2D NeverAccessedTexture;
layout(binding = 5) uniform sampler2D NeverAccessedSampler2D;

layout(location = 0) out vec4 color;

vec4 access_sampler_and_texture(texture2D t, sampler s, vec2 coord)
{
vec4 ret = texture(sampler2D(t, s), coord);
return vec4(5.0) * ret;
}

vec4 access_combined_sampler(sampler2D s)
{
vec2 coord = vec2(0.5, 0.5);
vec4 ret = texture(s, coord);
return vec4(1.0, 2.0, 3.0, 1.0) * ret;
}

vec4 call_access_functions(texture2D t, sampler s)
{
return access_combined_sampler(Sampler2D) + access_sampler_and_texture(t, s, vec2(0.25, 0.75));
}

vec4 never_called(texture2D t, sampler s, float u, float v)
{
vec4 ret = texture(sampler2D(t, s), vec2(u, v));
return vec4(-3.0) * ret;
}

vec4 never_called_2(vec2 coord)
{
vec4 ret = texture(sampler2D(NeverAccessedTexture, NeverAccessedSampler), coord);
ret *= texture(NeverAccessedSampler2D, coord);
return ret;
}

void main()
{
color = vec4(-1.0) * call_access_functions(Texture, Sampler);
}
Binary file added tests/issues/102/function_parameter_access.spv
Binary file not shown.
Loading

0 comments on commit f368a83

Please sign in to comment.