[Mesa-dev] [PATCH v2 2/2] glsl: raise warning when using uninitialized variables

[Jordan Justen]

On 2016-03-30 02:03:29, Alejandro Piñeiro wrote:
> On 30/03/16 09:44, Alejandro Piñeiro wrote:
> > On 30/03/16 06:16, Kenneth Graunke wrote:
> 
> >> I was just looking at that today...adding an ordinary global variable
> >> in builtin_variables.cpp seems really wrong.
> > I think that I don't follow.  Why gl_GlobalInvocationID is an ordinary
> > global variable? It is a built-in variable, and I assumed that all
> > built-ins would be using the ir_var_system_value ir_variable_mode. As
> > far as I understand this is not happening here due the lowering.
> 
> Ah ok, I think that I understand what you mean, now that I took a look
> to builtin_variables.cpp::1171. Yes gl_GlobalInvocationID and
> gl_LocalInvocationIndex are not added with add_system_value. As far as I
> see, it is because they are not included as system values at
> shader_enums.h:gl_system_value. I guess that because it is lowered as
> Illia mentioned. Jordan could you confirm the reason of adding those
> built-ins as regular variables?
> 

In v1, they were system values:

http://thread.gmane.org/gmane.comp.video.mesa3d.devel/109477

I think based on Ilia's feedback:

  " have you considered just starting out *every* CS with the
    instructions to compute them and then not having any lowering at
    all? "

I tried to handle the initialization of the variables differently. I
don't think Ilia's feedback really commented on if it was a system
value or not, so dropping the system value type was my idea/fault.

In v2, instead having a lowering pass that replaces the system value,
I inserted some code into main to initialize the variable.

http://thread.gmane.org/gmane.comp.video.mesa3d.devel/110920

I happen to think the glsl ir based init code looks better than the
system value lowering pass, but if we have some rule that all gl_*
variables must be system values, then I guess it was a bad choice.

I didn't think that this variable (and gl_LocalInvocationIndex) had a
real need to be turned into system values until there was a driver
that had a special way to initialize the variables which was better
than the result from the glsl ir based initialization.

Finally, it appears that Jason is adding these as a system value for
spir-v, which then gets lowered at the nir level. I think it would be
a reasonable idea to convert it to a system value, and not lower it in
glsl at all. This would then require the non-nir drivers to lower it
elsewhere. (In fact, this was suggested by Jason in v1 of the series,
but Ilia wanted it handled at the glsl level.)

-Jordan