<div dir="ltr"> <div class="gmail_quote"><div dir="ltr">On Tue, Nov 13, 2018 at 9:49 AM Karol Herbst <<a href="mailto:kherbst@redhat.com">kherbst@redhat.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">the naming is a bit confusing no matter how you look at it. Within OpenCL "global" memory is memory accessible from all threads. glsl "global" memory normally refers to shader thread private memory declared at global scope. As we already use "shared" for memory shared across all thrads of a work group the solution where everybody could be happy with is to rename "global" to "private" and use "global" later for memory usually stored within system accessible memory (be it VRAM or system RAM if keeping SVM in mind). </blockquote><div> </div><div>Yeah.... There's just no good way to pick these names. It's been kicking around in my brain for a while and I still don't have anything I like. One thing I will say, though, is that if we're going to rename global to private, I think I'd like to rename local to function at the same time. That way, it's clear that they map to the SPIR-V thing and we're no longer using the "global/local" terminology from C. If we do that, then I think I'm fine with this change.</div><div> </div><div>--Jason </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Signed-off-by: Karol Herbst <<a href="mailto:kherbst@redhat.com" target="_blank">kherbst@redhat.com</a>> --- src/compiler/glsl/glsl_to_nir.cpp | 4 ++-- src/compiler/nir/nir.c | 2 +- src/compiler/nir/nir.h | 2 +- src/compiler/nir/nir_linking_helpers.c | 2 +- .../nir/nir_lower_constant_initializers.c | 2 +- .../nir/nir_lower_global_vars_to_local.c | 4 ++-- src/compiler/nir/nir_lower_io_to_temporaries.c | 2 +- src/compiler/nir/nir_opt_copy_prop_vars.c | 4 ++-- src/compiler/nir/nir_opt_dead_write_vars.c | 2 +- src/compiler/nir/nir_print.c | 4 ++-- src/compiler/nir/nir_remove_dead_variables.c | 4 ++-- src/compiler/nir/nir_split_vars.c | 16 ++++++++-------- src/compiler/nir/tests/vars_tests.cpp | 2 +- src/compiler/spirv/vtn_private.h | 2 +- src/compiler/spirv/vtn_variables.c | 6 +++--- src/gallium/auxiliary/nir/tgsi_to_nir.c | 2 +- src/mesa/state_tracker/st_glsl_to_nir.cpp | 2 +- 17 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/compiler/glsl/glsl_to_nir.cpp b/src/compiler/glsl/glsl_to_nir.cpp index 0479f8fcfe4..8564cd89b5a 100644 --- a/src/compiler/glsl/glsl_to_nir.cpp +++ b/src/compiler/glsl/glsl_to_nir.cpp @@ -312,7 +312,7 @@ nir_visitor::visit(ir_variable *ir) case ir_var_auto: case ir_var_temporary: if (is_global) - var->data.mode = nir_var_global; + var->data.mode = nir_var_private; else var->data.mode = nir_var_local; break; @@ -1433,7 +1433,7 @@ nir_visitor::visit(ir_expression *ir) * sense, we'll just turn it into a load which will probably * eventually end up as an SSA definition. */ - assert(this->deref->mode == nir_var_global); + assert(this->deref->mode == nir_var_private); op = nir_intrinsic_load_deref; } diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c index 249b9357c3f..27f5d1b7bca 100644 --- a/src/compiler/nir/nir.c +++ b/src/compiler/nir/nir.c @@ -129,7 +129,7 @@ nir_shader_add_variable(nir_shader *shader, nir_variable *var) assert(!"nir_shader_add_variable cannot be used for local variables"); break; - case nir_var_global: + case nir_var_private: exec_list_push_tail(&shader->globals, &var->node); break; diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h index 89c28e36618..78f3204d3e2 100644 --- a/src/compiler/nir/nir.h +++ b/src/compiler/nir/nir.h @@ -96,7 +96,7 @@ typedef struct { typedef enum { nir_var_shader_in = (1 << 0), nir_var_shader_out = (1 << 1), - nir_var_global = (1 << 2), + nir_var_private = (1 << 2), nir_var_local = (1 << 3), nir_var_uniform = (1 << 4), nir_var_shader_storage = (1 << 5), diff --git a/src/compiler/nir/nir_linking_helpers.c b/src/compiler/nir/nir_linking_helpers.c index a05890ada43..d8358e08e5a 100644 --- a/src/compiler/nir/nir_linking_helpers.c +++ b/src/compiler/nir/nir_linking_helpers.c @@ -134,7 +134,7 @@ nir_remove_unused_io_vars(nir_shader *shader, struct exec_list *var_list, if (!(other_stage & get_variable_io_mask(var, shader->info.stage))) { /* This one is invalid, make it a global variable instead */ var->data.location = 0; - var->data.mode = nir_var_global; + var->data.mode = nir_var_private; exec_node_remove(&var->node); exec_list_push_tail(&shader->globals, &var->node); diff --git a/src/compiler/nir/nir_lower_constant_initializers.c b/src/compiler/nir/nir_lower_constant_initializers.c index 4e9cea46157..932a32b3c9c 100644 --- a/src/compiler/nir/nir_lower_constant_initializers.c +++ b/src/compiler/nir/nir_lower_constant_initializers.c @@ -98,7 +98,7 @@ nir_lower_constant_initializers(nir_shader *shader, nir_variable_mode modes) if (modes & nir_var_shader_out) progress |= lower_const_initializer(&builder, &shader->outputs); - if (modes & nir_var_global) + if (modes & nir_var_private) progress |= lower_const_initializer(&builder, &shader->globals); if (modes & nir_var_system_value) diff --git a/src/compiler/nir/nir_lower_global_vars_to_local.c b/src/compiler/nir/nir_lower_global_vars_to_local.c index be99cf9ad02..6c6d9a9d25c 100644 --- a/src/compiler/nir/nir_lower_global_vars_to_local.c +++ b/src/compiler/nir/nir_lower_global_vars_to_local.c @@ -36,7 +36,7 @@ static void register_var_use(nir_variable *var, nir_function_impl *impl, struct hash_table *var_func_table) { - if (var->data.mode != nir_var_global) + if (var->data.mode != nir_var_private) return; struct hash_entry *entry = @@ -89,7 +89,7 @@ nir_lower_global_vars_to_local(nir_shader *shader) nir_variable *var = (void *)entry->key; nir_function_impl *impl = entry->data; - assert(var->data.mode == nir_var_global); + assert(var->data.mode == nir_var_private); if (impl != NULL) { exec_node_remove(&var->node); diff --git a/src/compiler/nir/nir_lower_io_to_temporaries.c b/src/compiler/nir/nir_lower_io_to_temporaries.c index b83aaf46e6a..2487add33ed 100644 --- a/src/compiler/nir/nir_lower_io_to_temporaries.c +++ b/src/compiler/nir/nir_lower_io_to_temporaries.c @@ -134,7 +134,7 @@ create_shadow_temp(struct lower_io_state *state, nir_variable *var) /* Give the original a new name with @<mode>-temp appended */ const char *mode = (temp->data.mode == nir_var_shader_in) ? "in" : "out"; temp->name = ralloc_asprintf(var, "%s@%s-temp", mode, nvar->name); - temp->data.mode = nir_var_global; + temp->data.mode = nir_var_private; temp->data.read_only = false; temp->data.fb_fetch_output = false; temp->data.compact = false; diff --git a/src/compiler/nir/nir_opt_copy_prop_vars.c b/src/compiler/nir/nir_opt_copy_prop_vars.c index 48070cd5e31..95f0bbd5b77 100644 --- a/src/compiler/nir/nir_opt_copy_prop_vars.c +++ b/src/compiler/nir/nir_opt_copy_prop_vars.c @@ -134,7 +134,7 @@ gather_vars_written(struct copy_prop_var_state *state, nir_foreach_instr(instr, block) { if (instr->type == nir_instr_type_call) { written->modes |= nir_var_shader_out | - nir_var_global | + nir_var_private | nir_var_local | nir_var_shader_storage | nir_var_shared; @@ -603,7 +603,7 @@ copy_prop_vars_block(struct copy_prop_var_state *state, nir_foreach_instr_safe(instr, block) { if (instr->type == nir_instr_type_call) { apply_barrier_for_modes(copies, nir_var_shader_out | - nir_var_global | + nir_var_private | nir_var_local | nir_var_shader_storage | nir_var_shared); diff --git a/src/compiler/nir/nir_opt_dead_write_vars.c b/src/compiler/nir/nir_opt_dead_write_vars.c index dd949998cc8..0fbbe63f436 100644 --- a/src/compiler/nir/nir_opt_dead_write_vars.c +++ b/src/compiler/nir/nir_opt_dead_write_vars.c @@ -120,7 +120,7 @@ remove_dead_write_vars_local(void *mem_ctx, nir_block *block) nir_foreach_instr_safe(instr, block) { if (instr->type == nir_instr_type_call) { clear_unused_for_modes(&unused_writes, nir_var_shader_out | - nir_var_global | + nir_var_private | nir_var_local | nir_var_shader_storage | nir_var_shared); diff --git a/src/compiler/nir/nir_print.c b/src/compiler/nir/nir_print.c index 0de82800f8c..88f91087134 100644 --- a/src/compiler/nir/nir_print.c +++ b/src/compiler/nir/nir_print.c @@ -416,8 +416,8 @@ get_variable_mode_str(nir_variable_mode mode, bool want_local_global_mode) return "system"; case nir_var_shared: return "shared"; - case nir_var_global: - return want_local_global_mode ? "global" : ""; + case nir_var_private: + return want_local_global_mode ? "private" : ""; case nir_var_local: return want_local_global_mode ? "local" : ""; default: diff --git a/src/compiler/nir/nir_remove_dead_variables.c b/src/compiler/nir/nir_remove_dead_variables.c index fadc51a6977..d09e823c517 100644 --- a/src/compiler/nir/nir_remove_dead_variables.c +++ b/src/compiler/nir/nir_remove_dead_variables.c @@ -71,7 +71,7 @@ add_var_use_deref(nir_deref_instr *deref, struct set *live) * all means we need to keep it alive. */ assert(deref->mode == deref->var->data.mode); - if (!(deref->mode & (nir_var_local | nir_var_global | nir_var_shared)) || + if (!(deref->mode & (nir_var_local | nir_var_private | nir_var_shared)) || deref_used_for_not_store(deref)) _mesa_set_add(live, deref->var); } @@ -175,7 +175,7 @@ nir_remove_dead_variables(nir_shader *shader, nir_variable_mode modes) if (modes & nir_var_shader_out) progress = remove_dead_vars(&shader->outputs, live) || progress; - if (modes & nir_var_global) + if (modes & nir_var_private) progress = remove_dead_vars(&shader->globals, live) || progress; if (modes & nir_var_system_value) diff --git a/src/compiler/nir/nir_split_vars.c b/src/compiler/nir/nir_split_vars.c index bf9205c5150..9d1a096268c 100644 --- a/src/compiler/nir/nir_split_vars.c +++ b/src/compiler/nir/nir_split_vars.c @@ -259,10 +259,10 @@ nir_split_struct_vars(nir_shader *shader, nir_variable_mode modes) _mesa_hash_table_create(mem_ctx, _mesa_hash_pointer, _mesa_key_pointer_equal); - assert((modes & (nir_var_global | nir_var_local)) == modes); + assert((modes & (nir_var_private | nir_var_local)) == modes); bool has_global_splits = false; - if (modes & nir_var_global) { + if (modes & nir_var_private) { has_global_splits = split_var_list_structs(shader, NULL, &shader->globals, var_field_map, mem_ctx); @@ -795,10 +795,10 @@ nir_split_array_vars(nir_shader *shader, nir_variable_mode modes) _mesa_hash_table_create(mem_ctx, _mesa_hash_pointer, _mesa_key_pointer_equal); - assert((modes & (nir_var_global | nir_var_local)) == modes); + assert((modes & (nir_var_private | nir_var_local)) == modes); bool has_global_array = false; - if (modes & nir_var_global) { + if (modes & nir_var_private) { has_global_array = init_var_list_array_infos(&shader->globals, var_info_map, mem_ctx); } @@ -827,7 +827,7 @@ nir_split_array_vars(nir_shader *shader, nir_variable_mode modes) } bool has_global_splits = false; - if (modes & nir_var_global) { + if (modes & nir_var_private) { has_global_splits = split_var_list_arrays(shader, NULL, &shader->globals, var_info_map, mem_ctx); @@ -1494,7 +1494,7 @@ function_impl_has_vars_with_modes(nir_function_impl *impl, { nir_shader *shader = impl->function->shader; - if ((modes & nir_var_global) && !exec_list_is_empty(&shader->globals)) + if ((modes & nir_var_private) && !exec_list_is_empty(&shader->globals)) return true; if ((modes & nir_var_local) && !exec_list_is_empty(&impl->locals)) @@ -1515,7 +1515,7 @@ function_impl_has_vars_with_modes(nir_function_impl *impl, bool nir_shrink_vec_array_vars(nir_shader *shader, nir_variable_mode modes) { - assert((modes & (nir_var_global | nir_var_local)) == modes); + assert((modes & (nir_var_private | nir_var_local)) == modes); void *mem_ctx = ralloc_context(NULL); @@ -1544,7 +1544,7 @@ nir_shrink_vec_array_vars(nir_shader *shader, nir_variable_mode modes) } bool globals_shrunk = false; - if (modes & nir_var_global) + if (modes & nir_var_private) globals_shrunk = shrink_vec_var_list(&shader->globals, var_usage_map); bool progress = false; diff --git a/src/compiler/nir/tests/vars_tests.cpp b/src/compiler/nir/tests/vars_tests.cpp index 32763d2db64..666d22643dd 100644 --- a/src/compiler/nir/tests/vars_tests.cpp +++ b/src/compiler/nir/tests/vars_tests.cpp @@ -191,7 +191,7 @@ TEST_F(nir_redundant_load_vars_test, invalidate_inside_if_block) * if statement. They should be invalidated accordingly. */ - nir_variable **g = create_many_int(nir_var_global, "g", 3); + nir_variable **g = create_many_int(nir_var_private, "g", 3); nir_variable **out = create_many_int(nir_var_shader_out, "out", 3); nir_load_var(b, g[0]); diff --git a/src/compiler/spirv/vtn_private.h b/src/compiler/spirv/vtn_private.h index d7d5b8db198..86f98083f58 100644 --- a/src/compiler/spirv/vtn_private.h +++ b/src/compiler/spirv/vtn_private.h @@ -418,7 +418,7 @@ struct vtn_access_chain { enum vtn_variable_mode { vtn_variable_mode_local, - vtn_variable_mode_global, + vtn_variable_mode_private, vtn_variable_mode_uniform, vtn_variable_mode_ubo, vtn_variable_mode_ssbo, diff --git a/src/compiler/spirv/vtn_variables.c b/src/compiler/spirv/vtn_variables.c index e7654b768af..5738941ffb6 100644 --- a/src/compiler/spirv/vtn_variables.c +++ b/src/compiler/spirv/vtn_variables.c @@ -1556,8 +1556,8 @@ vtn_storage_class_to_mode(struct vtn_builder *b, nir_mode = nir_var_shader_out; break; case SpvStorageClassPrivate: - mode = vtn_variable_mode_global; - nir_mode = nir_var_global; + mode = vtn_variable_mode_private; + nir_mode = nir_var_private; break; case SpvStorageClassFunction: mode = vtn_variable_mode_local; @@ -1721,7 +1721,7 @@ vtn_create_variable(struct vtn_builder *b, struct vtn_value *val, switch (var->mode) { case vtn_variable_mode_local: - case vtn_variable_mode_global: + case vtn_variable_mode_private: case vtn_variable_mode_uniform: /* For these, we create the variable normally */ var->var = rzalloc(b->shader, nir_variable); diff --git a/src/gallium/auxiliary/nir/tgsi_to_nir.c b/src/gallium/auxiliary/nir/tgsi_to_nir.c index b108c05d895..c95c45279f3 100644 --- a/src/gallium/auxiliary/nir/tgsi_to_nir.c +++ b/src/gallium/auxiliary/nir/tgsi_to_nir.c @@ -182,7 +182,7 @@ ttn_emit_declaration(struct ttn_compile *c) nir_variable *var = rzalloc(b->shader, nir_variable); var->type = glsl_array_type(glsl_vec4_type(), array_size); - var->data.mode = nir_var_global; + var->data.mode = nir_var_private; var->name = ralloc_asprintf(var, "arr_%d", decl->Array.ArrayID); exec_list_push_tail(&b->shader->globals, &var->node); diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp b/src/mesa/state_tracker/st_glsl_to_nir.cpp index d0475fb538a..0152fe6d7ff 100644 --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp @@ -103,7 +103,7 @@ st_nir_assign_vs_in_locations(struct gl_program *prog, nir_shader *nir) * set. */ exec_node_remove(&var->node); - var->data.mode = nir_var_global; + var->data.mode = nir_var_private; exec_list_push_tail(&nir->globals, &var->node); } } -- 2.19.1 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div></div>