On Tue, Nov 13, 2018 at 9:49 AM Karol Herbst <kher...@redhat.com> wrote:
> 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).
>
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.
--Jason
> Signed-off-by: Karol Herbst <kher...@redhat.com>
> ---
> 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
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
