<div dir="ltr"> <div class="gmail_quote"><div dir="ltr">On Fri, Jan 11, 2019 at 1:50 PM Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Friday, January 11, 2019 9:32:20 AM PST Eric Anholt wrote: > Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> writes: > > > On Fri, Jan 11, 2019 at 11:11 AM Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>> > > wrote: > > > >> On Friday, January 11, 2019 8:33:41 AM PST Jason Ekstrand wrote: > >> > On Fri, Jan 11, 2019 at 10:19 AM Kenneth Graunke wrote: > >> > > 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) > >> > > > >> > > >> > Are those flipped? > >> > >> Gah! Sorry. Yes. > >> > >> nir_var_shader_in > >> nir_var_shader_out > >> nir_var_shader_temp (formerly global/private) > >> nir_var_local_temp (formerly local/function) > >> 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) > >> > > > > I can work with that. I do think I'd mildly prefer function_temp over > > local_temp but I think the adding of _temp is an improvement. > > I like the _temp suggestion a lot! And I think I'm also mildly in favor > of function_temp. > > (Also, thanks to taking naming seriously, to everyone involved here. > It's hard.) Yep, it's not easy. Karol, sorry for being grumpy the other day, it wasn't a very constructive email on my part. I agree that the names should change, for all the reasons you suggested. I'm fine with nir_var_function_temp / nir_var_func_temp if people prefer that to local_temp. Closer to Karol's original suggestion, but I think "temp" clarifies it a bit. shader_in is an input in the shader, function_temp is a temp in a function. Sound good? Should I do the sed-job and send patches? </blockquote><div> </div><div>Go for it. Mind also renaming the variable lists? I don't care if it's one patch or two. </div></div></div>