From a700a82bdac19433533ccf31ab635350cb58203b Mon Sep 17 00:00:00 2001 From: Jason Ekstrand Date: Fri, 14 Dec 2018 11:21:50 -0600 Subject: [PATCH] nir: Distinguish between normal uniforms and UBOs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, NIR had a single nir_var_uniform mode used for atomic counters, UBOs, samplers, images, and normal uniforms. This commit splits this into nir_var_uniform and nir_var_ubo where nir_var_uniform is still a bit of a catch-all but the nir_var_ubo is specific to UBOs. While we're at it, we also rename shader_storage to ssbo to follow the convention. We need this so that we can distinguish between normal uniforms and UBO access at the deref level without going all the way back variable and seeing if it has an interface type. Reviewed-by: Alejandro Piñeiro Reviewed-by: Caio Marcelo de Oliveira Filho --- src/compiler/glsl/gl_nir_lower_atomics.c | 2 +- src/compiler/glsl/glsl_to_nir.cpp | 7 ++++-- src/compiler/nir/nir.c | 3 ++- src/compiler/nir/nir.h | 3 ++- src/compiler/nir/nir_lower_atomics_to_ssbo.c | 3 +-- src/compiler/nir/nir_lower_io.c | 3 +-- src/compiler/nir/nir_opt_copy_prop_vars.c | 8 +++--- src/compiler/nir/nir_opt_dead_write_vars.c | 4 +-- src/compiler/nir/nir_print.c | 9 ++++--- src/compiler/nir/tests/vars_tests.cpp | 26 ++++++++++---------- src/compiler/spirv/vtn_variables.c | 6 ++--- src/gallium/drivers/radeonsi/si_shader_nir.c | 5 ++-- src/mesa/state_tracker/st_glsl_to_nir.cpp | 3 +-- 13 files changed, 44 insertions(+), 38 deletions(-) diff --git a/src/compiler/glsl/gl_nir_lower_atomics.c b/src/compiler/glsl/gl_nir_lower_atomics.c index 36e273c45d2..e0df547416f 100644 --- a/src/compiler/glsl/gl_nir_lower_atomics.c +++ b/src/compiler/glsl/gl_nir_lower_atomics.c @@ -101,7 +101,7 @@ lower_deref_instr(nir_builder *b, nir_intrinsic_instr *instr, nir_variable *var = nir_deref_instr_get_variable(deref); if (var->data.mode != nir_var_uniform && - var->data.mode != nir_var_shader_storage && + var->data.mode != nir_var_ssbo && var->data.mode != nir_var_shared) return false; /* atomics passed as function arguments can't be lowered */ diff --git a/src/compiler/glsl/glsl_to_nir.cpp b/src/compiler/glsl/glsl_to_nir.cpp index c5ba47d9e30..d1a051b1d18 100644 --- a/src/compiler/glsl/glsl_to_nir.cpp +++ b/src/compiler/glsl/glsl_to_nir.cpp @@ -354,11 +354,14 @@ nir_visitor::visit(ir_variable *ir) break; case ir_var_uniform: - var->data.mode = nir_var_uniform; + if (ir->get_interface_type()) + var->data.mode = nir_var_ubo; + else + var->data.mode = nir_var_uniform; break; case ir_var_shader_storage: - var->data.mode = nir_var_shader_storage; + var->data.mode = nir_var_ssbo; break; case ir_var_system_value: diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c index 3c80e03a091..b0b031cde61 100644 --- a/src/compiler/nir/nir.c +++ b/src/compiler/nir/nir.c @@ -142,7 +142,8 @@ nir_shader_add_variable(nir_shader *shader, nir_variable *var) break; case nir_var_uniform: - case nir_var_shader_storage: + case nir_var_ubo: + case nir_var_ssbo: exec_list_push_tail(&shader->uniforms, &var->node); break; diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h index 497327eaca8..e72585000d4 100644 --- a/src/compiler/nir/nir.h +++ b/src/compiler/nir/nir.h @@ -100,8 +100,9 @@ typedef enum { nir_var_global = (1 << 2), nir_var_local = (1 << 3), nir_var_uniform = (1 << 4), - nir_var_shader_storage = (1 << 5), + nir_var_ubo = (1 << 5), nir_var_system_value = (1 << 6), + nir_var_ssbo = (1 << 7), nir_var_shared = (1 << 8), nir_var_all = ~0, } nir_variable_mode; diff --git a/src/compiler/nir/nir_lower_atomics_to_ssbo.c b/src/compiler/nir/nir_lower_atomics_to_ssbo.c index cdc660981fc..d9acc5c3a79 100644 --- a/src/compiler/nir/nir_lower_atomics_to_ssbo.c +++ b/src/compiler/nir/nir_lower_atomics_to_ssbo.c @@ -223,8 +223,7 @@ nir_lower_atomics_to_ssbo(nir_shader *shader, unsigned ssbo_offset) snprintf(name, sizeof(name), "counter%d", var->data.binding); - ssbo = nir_variable_create(shader, nir_var_shader_storage, - type, name); + ssbo = nir_variable_create(shader, nir_var_ssbo, type, name); ssbo->data.binding = var->data.binding; struct glsl_struct_field field = { diff --git a/src/compiler/nir/nir_lower_io.c b/src/compiler/nir/nir_lower_io.c index 64424307812..2ccba8c032b 100644 --- a/src/compiler/nir/nir_lower_io.c +++ b/src/compiler/nir/nir_lower_io.c @@ -54,8 +54,7 @@ nir_assign_var_locations(struct exec_list *var_list, unsigned *size, * UBOs have their own address spaces, so don't count them towards the * number of global uniforms */ - if ((var->data.mode == nir_var_uniform || var->data.mode == nir_var_shader_storage) && - var->interface_type != NULL) + if (var->data.mode == nir_var_ubo || var->data.mode == nir_var_ssbo) continue; var->data.driver_location = location; diff --git a/src/compiler/nir/nir_opt_copy_prop_vars.c b/src/compiler/nir/nir_opt_copy_prop_vars.c index e109814abcd..36403e87e8f 100644 --- a/src/compiler/nir/nir_opt_copy_prop_vars.c +++ b/src/compiler/nir/nir_opt_copy_prop_vars.c @@ -136,7 +136,7 @@ gather_vars_written(struct copy_prop_var_state *state, written->modes |= nir_var_shader_out | nir_var_global | nir_var_local | - nir_var_shader_storage | + nir_var_ssbo | nir_var_shared; continue; } @@ -149,7 +149,7 @@ gather_vars_written(struct copy_prop_var_state *state, case nir_intrinsic_barrier: case nir_intrinsic_memory_barrier: written->modes |= nir_var_shader_out | - nir_var_shader_storage | + nir_var_ssbo | nir_var_shared; break; @@ -617,7 +617,7 @@ copy_prop_vars_block(struct copy_prop_var_state *state, apply_barrier_for_modes(copies, nir_var_shader_out | nir_var_global | nir_var_local | - nir_var_shader_storage | + nir_var_ssbo | nir_var_shared); continue; } @@ -630,7 +630,7 @@ copy_prop_vars_block(struct copy_prop_var_state *state, case nir_intrinsic_barrier: case nir_intrinsic_memory_barrier: apply_barrier_for_modes(copies, nir_var_shader_out | - nir_var_shader_storage | + nir_var_ssbo | nir_var_shared); break; diff --git a/src/compiler/nir/nir_opt_dead_write_vars.c b/src/compiler/nir/nir_opt_dead_write_vars.c index 75814738d84..2ae5f78b791 100644 --- a/src/compiler/nir/nir_opt_dead_write_vars.c +++ b/src/compiler/nir/nir_opt_dead_write_vars.c @@ -121,7 +121,7 @@ remove_dead_write_vars_local(void *mem_ctx, nir_block *block) clear_unused_for_modes(&unused_writes, nir_var_shader_out | nir_var_global | nir_var_local | - nir_var_shader_storage | + nir_var_ssbo | nir_var_shared); continue; } @@ -134,7 +134,7 @@ remove_dead_write_vars_local(void *mem_ctx, nir_block *block) case nir_intrinsic_barrier: case nir_intrinsic_memory_barrier: { clear_unused_for_modes(&unused_writes, nir_var_shader_out | - nir_var_shader_storage | + nir_var_ssbo | nir_var_shared); break; } diff --git a/src/compiler/nir/nir_print.c b/src/compiler/nir/nir_print.c index 5f1b547ea37..95d9bc85656 100644 --- a/src/compiler/nir/nir_print.c +++ b/src/compiler/nir/nir_print.c @@ -412,10 +412,12 @@ get_variable_mode_str(nir_variable_mode mode, bool want_local_global_mode) return "shader_out"; case nir_var_uniform: return "uniform"; - case nir_var_shader_storage: - return "shader_storage"; + case nir_var_ubo: + return "ubo"; case nir_var_system_value: return "system"; + case nir_var_ssbo: + return "ssbo"; case nir_var_shared: return "shared"; case nir_var_global: @@ -503,7 +505,8 @@ print_var_decl(nir_variable *var, print_state *state) if (var->data.mode == nir_var_shader_in || var->data.mode == nir_var_shader_out || var->data.mode == nir_var_uniform || - var->data.mode == nir_var_shader_storage) { + var->data.mode == nir_var_ubo || + var->data.mode == nir_var_ssbo) { const char *loc = NULL; char buf[4]; diff --git a/src/compiler/nir/tests/vars_tests.cpp b/src/compiler/nir/tests/vars_tests.cpp index 32763d2db64..74bf5c05023 100644 --- a/src/compiler/nir/tests/vars_tests.cpp +++ b/src/compiler/nir/tests/vars_tests.cpp @@ -237,7 +237,7 @@ TEST_F(nir_redundant_load_vars_test, invalidate_live_load_in_the_end_of_loop) * body. */ - nir_variable *v = create_int(nir_var_shader_storage, "v"); + nir_variable *v = create_int(nir_var_ssbo, "v"); nir_load_var(b, v); @@ -433,7 +433,7 @@ TEST_F(nir_copy_prop_vars_test, store_store_load_different_components_in_many_bl TEST_F(nir_copy_prop_vars_test, memory_barrier_in_two_blocks) { - nir_variable **v = create_many_int(nir_var_shader_storage, "v", 4); + nir_variable **v = create_many_int(nir_var_ssbo, "v", 4); nir_store_var(b, v[0], nir_imm_int(b, 1), 1); nir_store_var(b, v[1], nir_imm_int(b, 2), 1); @@ -490,7 +490,7 @@ TEST_F(nir_copy_prop_vars_test, simple_store_load_in_two_blocks) TEST_F(nir_dead_write_vars_test, no_dead_writes_in_block) { - nir_variable **v = create_many_int(nir_var_shader_storage, "v", 2); + nir_variable **v = create_many_int(nir_var_ssbo, "v", 2); nir_store_var(b, v[0], nir_load_var(b, v[1]), 1); @@ -500,7 +500,7 @@ TEST_F(nir_dead_write_vars_test, no_dead_writes_in_block) TEST_F(nir_dead_write_vars_test, no_dead_writes_different_components_in_block) { - nir_variable **v = create_many_ivec2(nir_var_shader_storage, "v", 3); + nir_variable **v = create_many_ivec2(nir_var_ssbo, "v", 3); nir_store_var(b, v[0], nir_load_var(b, v[1]), 1 << 0); nir_store_var(b, v[0], nir_load_var(b, v[2]), 1 << 1); @@ -511,7 +511,7 @@ TEST_F(nir_dead_write_vars_test, no_dead_writes_different_components_in_block) TEST_F(nir_dead_write_vars_test, no_dead_writes_in_if_statement) { - nir_variable **v = create_many_int(nir_var_shader_storage, "v", 6); + nir_variable **v = create_many_int(nir_var_ssbo, "v", 6); nir_store_var(b, v[2], nir_load_var(b, v[0]), 1); nir_store_var(b, v[3], nir_load_var(b, v[1]), 1); @@ -531,7 +531,7 @@ TEST_F(nir_dead_write_vars_test, no_dead_writes_in_if_statement) TEST_F(nir_dead_write_vars_test, no_dead_writes_in_loop_statement) { - nir_variable **v = create_many_int(nir_var_shader_storage, "v", 3); + nir_variable **v = create_many_int(nir_var_ssbo, "v", 3); nir_store_var(b, v[0], nir_load_var(b, v[1]), 1); @@ -553,7 +553,7 @@ TEST_F(nir_dead_write_vars_test, no_dead_writes_in_loop_statement) TEST_F(nir_dead_write_vars_test, dead_write_in_block) { - nir_variable **v = create_many_int(nir_var_shader_storage, "v", 3); + nir_variable **v = create_many_int(nir_var_ssbo, "v", 3); nir_store_var(b, v[0], nir_load_var(b, v[1]), 1); nir_ssa_def *load_v2 = nir_load_var(b, v[2]); @@ -571,7 +571,7 @@ TEST_F(nir_dead_write_vars_test, dead_write_in_block) TEST_F(nir_dead_write_vars_test, dead_write_components_in_block) { - nir_variable **v = create_many_ivec2(nir_var_shader_storage, "v", 3); + nir_variable **v = create_many_ivec2(nir_var_ssbo, "v", 3); nir_store_var(b, v[0], nir_load_var(b, v[1]), 1 << 0); nir_ssa_def *load_v2 = nir_load_var(b, v[2]); @@ -595,7 +595,7 @@ TEST_F(nir_dead_write_vars_test, dead_write_components_in_block) TEST_F(nir_dead_write_vars_test, DISABLED_dead_write_in_two_blocks) { - nir_variable **v = create_many_int(nir_var_shader_storage, "v", 3); + nir_variable **v = create_many_int(nir_var_ssbo, "v", 3); nir_store_var(b, v[0], nir_load_var(b, v[1]), 1); nir_ssa_def *load_v2 = nir_load_var(b, v[2]); @@ -617,7 +617,7 @@ TEST_F(nir_dead_write_vars_test, DISABLED_dead_write_in_two_blocks) TEST_F(nir_dead_write_vars_test, DISABLED_dead_write_components_in_two_blocks) { - nir_variable **v = create_many_ivec2(nir_var_shader_storage, "v", 3); + nir_variable **v = create_many_ivec2(nir_var_ssbo, "v", 3); nir_store_var(b, v[0], nir_load_var(b, v[1]), 1 << 0); @@ -639,7 +639,7 @@ TEST_F(nir_dead_write_vars_test, DISABLED_dead_write_components_in_two_blocks) TEST_F(nir_dead_write_vars_test, DISABLED_dead_writes_in_if_statement) { - nir_variable **v = create_many_int(nir_var_shader_storage, "v", 4); + nir_variable **v = create_many_int(nir_var_ssbo, "v", 4); /* Both branches will overwrite, making the previous store dead. */ nir_store_var(b, v[0], nir_load_var(b, v[1]), 1); @@ -670,7 +670,7 @@ TEST_F(nir_dead_write_vars_test, DISABLED_dead_writes_in_if_statement) TEST_F(nir_dead_write_vars_test, DISABLED_memory_barrier_in_two_blocks) { - nir_variable **v = create_many_int(nir_var_shader_storage, "v", 2); + nir_variable **v = create_many_int(nir_var_ssbo, "v", 2); nir_store_var(b, v[0], nir_imm_int(b, 1), 1); nir_store_var(b, v[1], nir_imm_int(b, 2), 1); @@ -693,7 +693,7 @@ TEST_F(nir_dead_write_vars_test, DISABLED_memory_barrier_in_two_blocks) TEST_F(nir_dead_write_vars_test, DISABLED_unrelated_barrier_in_two_blocks) { - nir_variable **v = create_many_int(nir_var_shader_storage, "v", 3); + nir_variable **v = create_many_int(nir_var_ssbo, "v", 3); nir_variable *out = create_int(nir_var_shader_out, "out"); nir_store_var(b, out, nir_load_var(b, v[1]), 1); diff --git a/src/compiler/spirv/vtn_variables.c b/src/compiler/spirv/vtn_variables.c index 70bec69a052..f33eb5509a0 100644 --- a/src/compiler/spirv/vtn_variables.c +++ b/src/compiler/spirv/vtn_variables.c @@ -1497,10 +1497,10 @@ vtn_storage_class_to_mode(struct vtn_builder *b, case SpvStorageClassUniform: if (interface_type->block) { mode = vtn_variable_mode_ubo; - nir_mode = 0; + nir_mode = nir_var_ubo; } else if (interface_type->buffer_block) { mode = vtn_variable_mode_ssbo; - nir_mode = 0; + nir_mode = nir_var_ssbo; } else { /* Default-block uniforms, coming from gl_spirv */ mode = vtn_variable_mode_uniform; @@ -1509,7 +1509,7 @@ vtn_storage_class_to_mode(struct vtn_builder *b, break; case SpvStorageClassStorageBuffer: mode = vtn_variable_mode_ssbo; - nir_mode = 0; + nir_mode = nir_var_ssbo; break; case SpvStorageClassUniformConstant: mode = vtn_variable_mode_uniform; diff --git a/src/gallium/drivers/radeonsi/si_shader_nir.c b/src/gallium/drivers/radeonsi/si_shader_nir.c index 4c6eb8ec808..64acf41679b 100644 --- a/src/gallium/drivers/radeonsi/si_shader_nir.c +++ b/src/gallium/drivers/radeonsi/si_shader_nir.c @@ -675,7 +675,8 @@ void si_nir_scan_shader(const struct nir_shader *nir, * so we don't need to worry about the ordering. */ if (variable->interface_type != NULL) { - if (variable->data.mode == nir_var_uniform) { + if (variable->data.mode == nir_var_uniform || + variable->data.mode == nir_var_ubo) { unsigned block_count; if (base_type != GLSL_TYPE_INTERFACE) { @@ -699,7 +700,7 @@ void si_nir_scan_shader(const struct nir_shader *nir, _mesa_set_add(ubo_set, variable->interface_type); } - if (variable->data.mode == nir_var_shader_storage) { + if (variable->data.mode == nir_var_ssbo) { /* TODO: make this more accurate */ info->shader_buffers_declared = u_bit_consecutive(0, SI_NUM_SHADER_BUFFERS); diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp b/src/mesa/state_tracker/st_glsl_to_nir.cpp index cbce4661e92..5d74524be79 100644 --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp @@ -253,8 +253,7 @@ st_nir_assign_uniform_locations(struct gl_context *ctx, * UBO's have their own address spaces, so don't count them towards the * number of global uniforms */ - if ((uniform->data.mode == nir_var_uniform || uniform->data.mode == nir_var_shader_storage) && - uniform->interface_type != NULL) + if (uniform->data.mode == nir_var_ubo || uniform->data.mode == nir_var_ssbo) continue; const struct glsl_type *type = glsl_without_array(uniform->type);