[Mesa-dev] [WIP 11/13] glsl: parser changes for GL_ARB_explicit_uniform_location

[Tapani Pälli]

On 04/05/2014 12:58 AM, Ian Romanick wrote:
> On 04/04/2014 02:42 PM, Ian Romanick wrote:
>> On 03/27/2014 11:45 PM, Tapani Pälli wrote:
>>> Patch adds a preprocessor define for the extension and stores
>>> explicit location data for uniforms during AST->HIR conversion.
>>> It also sets layout token to be available when having the
>>> extension in place.
>>>
>>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>>> ---
>>>  src/glsl/ast_to_hir.cpp      | 13 +++++++++++++
>>>  src/glsl/glcpp/glcpp-parse.y |  3 +++
>>>  src/glsl/glsl_lexer.ll       |  1 +
>>>  3 files changed, 17 insertions(+)
>>>
>>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>>> index 8f6e901..3bfad02 100644
>>> --- a/src/glsl/ast_to_hir.cpp
>>> +++ b/src/glsl/ast_to_hir.cpp
>>> @@ -2123,6 +2123,19 @@ validate_explicit_location(const struct ast_type_qualifier *qual,
>>>  {
>>>     bool fail = false;
>>>  
>>> +   /* Checks for GL_ARB_explicit_uniform_location. */
>>> +   if (qual->flags.q.uniform) {
>>> +
>>> +      if (qual->index < 0) {
>>> +         _mesa_glsl_error(loc, state,
>>> +                          "explicit location < 0 for uniform %s", var->name);
>> We also need to check that the explicit location isn't too large.  I
>> don't think that check can happen here because (I think) it will depend
>> on the size of the uniform.  The spec isn't completely clear, but I think
>>
>> // assume GL_MAX_UNIFORM_LOCATIONS is 65536
>> layout(location=65535) float foo[1];
>>
>> should fail because foo[1] would have location 65536 (larger than
>> GL_MAX_UNIFORM_LOCATIONS-1).
>>
>> Given the other rules in the spec about counting and use of locations, I
>> think ast_to_hir is the right time to do the upper bound check.  I
>> believe we can figure out how many locations a structure or array
>> uniform will maximally use at that point.
> I now see that you added that check in patch 8.  Maybe just put a
> comment here explaining why the check can't happen here.

OK, I'll try to move check for bounds to happen while parsing to be able
to fail earlier. I originally preferred to have all checks in one place
and location assignment felt like the right spot.

>>> +      } else {
>>> +         var->data.explicit_location = true;
>>> +         var->data.location = qual->location;
>>> +      }
>>> +      return;
>>> +   }
>>> +
>>>     /* Between GL_ARB_explicit_attrib_location an
>>>      * GL_ARB_separate_shader_objects, the inputs and outputs of any shader
>>>      * stage can be assigned explicit locations.  The checking here associates
>>> diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
>>> index f28d853..6d42138 100644
>>> --- a/src/glsl/glcpp/glcpp-parse.y
>>> +++ b/src/glsl/glcpp/glcpp-parse.y
>>> @@ -2087,6 +2087,9 @@ _glcpp_parser_handle_version_declaration(glcpp_parser_t *parser, intmax_t versio
>>>  	      if (extensions->ARB_explicit_attrib_location)
>>>  	         add_builtin_define(parser, "GL_ARB_explicit_attrib_location", 1);
>>>  
>>> +	      if (extensions->ARB_explicit_uniform_location)
>>> +	         add_builtin_define(parser, "GL_ARB_explicit_uniform_location", 1);
>>> +
>>>  	      if (extensions->ARB_shader_texture_lod)
>>>  	         add_builtin_define(parser, "GL_ARB_shader_texture_lod", 1);
>>>  
>>> diff --git a/src/glsl/glsl_lexer.ll b/src/glsl/glsl_lexer.ll
>>> index 7602351..83f0b6d 100644
>>> --- a/src/glsl/glsl_lexer.ll
>>> +++ b/src/glsl/glsl_lexer.ll
>>> @@ -393,6 +393,7 @@ layout		{
>>>  		      || yyextra->AMD_conservative_depth_enable
>>>  		      || yyextra->ARB_conservative_depth_enable
>>>  		      || yyextra->ARB_explicit_attrib_location_enable
>>> +		      || yyextra->ARB_explicit_uniform_location_enable
>>>                        || yyextra->has_separate_shader_objects()
>>>  		      || yyextra->ARB_uniform_buffer_object_enable
>>>  		      || yyextra->ARB_fragment_coord_conventions_enable
>>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>