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 <kher...@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
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
