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

[Kenneth Graunke]

On Thursday, March 31, 2016 12:37:31 AM PDT Jordan Justen wrote:
> 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

Ahh.  Thanks.  That makes sense, it was just a bit surprising to see.

It's probably best to leave this as is, and just add !is_gl_identifier()
to Alejandro's warning code as Ilia suggested.  That'll ensure every
built-in value is skipped.

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