Re: [Mesa-dev] [PATCH 17/22] nir: rename global to private memory
On Tue, Nov 13, 2018 at 9:49 AM Karol Herbst 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
> ---
> 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(>globals, >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(>node);
> exec_list_push_tail(>globals, >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
> ---
[Mesa-dev] [PATCH 17/22] nir: rename global to private memory
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).
Signed-off-by: Karol Herbst
---
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(>globals, >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(>node);
exec_list_push_tail(>globals, >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(, >outputs);
- if (modes & nir_var_global)
+ if (modes & nir_var_private)
progress |= lower_const_initializer(, >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
