[Mesa-dev] [PATCH 05/12] nir: rename global/local to private/function memory

[Kenneth Graunke]

On Wednesday, January 9, 2019 5:33:22 PM PST Ian Romanick wrote:
> On 1/8/19 9:57 PM, Kenneth Graunke wrote:
> > On Tuesday, December 4, 2018 10:26:43 AM PST Karol Herbst wrote:
> >> the naming is a bit confusing no matter how you look at it. Within SPIR-V
> >> "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).
> >> glsl "local" memory is memory only accessible within a function, while SPIR-V
> >> "local" memory is memory accessible within the same workgroup.
> >>
> >> v2: rename local to function as well
> >>
> >> Signed-off-by: Karol Herbst <kherbst at redhat.com>
> > 
> > I strongly dislike this patch, and I think we ought to revert it.
> > 
> > This probably makes sense from an OpenCL memory-model view of the world,
> > but it's really confusing from a compiler or general programming point
> > of view.
> > 
> > /Everybody/ knows what a local variable is.  It's one of the most used
> > concepts in programming.  Calling it nir_var_function is very confusing.
> > The variable is a...function?  Maybe it's a function pointer?  Neither
> > of those things even exist in GLSL, so...what the heck is it?
> > 
> > Renaming global scope variables to "private" is also confusing IMO.
> > They're certainly not private to a function.  They're globally
> > accessible by anything in the whole shader.  I'll admit "global" isn't
> > a great name either.
> 
> It seems like the concepts we're after a function local and thread
> local, so why not nir_var_thread_local (for old nir_var_global) and
> nir_var_function_local (for old nir_var_local).  When "global" is
> reintroduced to mean thread global, we could add it as
> nir_var_thread_global.  That seems to match at least one reasonable view
> of a storage hierarchy.

Those names (nir_var_func_local, nir_var_thread_local, and
nir_var_thread_global) make more sense to me than private/function.

Another option is `nir_var_local_temp` and `nir_var_shader_temp`,
indicating that they're just temporary variables, and not anything
with special semantics like memory.  shader_temp would pair well with
the existing shader_in/shader_out, since they have the same scope.

I might also consider adding 'mem' to variables representing memory.

So that would look like...

   nir_var_shader_in
   nir_var_shader_out
   nir_var_shader_temp  (formerly local/function)
   nir_var_local_temp   (formerly global/private)
   nir_var_uniform
   nir_var_system_value
   nir_var_mem_ubo      (added mem)
   nir_var_mem_ssbo     (added mem)
   nir_var_mem_shared   (added mem)
   nir_var_mem_global   (the new global memory type being introduced)

How does that look?

We may also want to rename the nir->globals list, or
nir_lower_global_vars_to_local and nir_opt_global_to_local.  Not sure.

--Ken
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190111/3a7870b1/attachment.sig>