[Mesa-dev] [PATCH 1/2] glapi: add KHR_no_error support to dispatch table generation

[Timothy Arceri]

On 02/05/17 20:43, Nicolai Hähnle wrote:
> On 27.04.2017 07:33, Timothy Arceri wrote:
>> This will allows us to create no error versions of functions
>> noted by a _no_error suffix. We also need to set a no_error
>> attribute equal to "true" in the xml.
>> ---
>>  src/mapi/glapi/gen/gl_XML.py     |  5 +++++
>>  src/mapi/glapi/gen/gl_genexec.py | 19 ++++++++++++++-----
>>  2 files changed, 19 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/mapi/glapi/gen/gl_XML.py b/src/mapi/glapi/gen/gl_XML.py
>> index c688906..603ef73 100644
>> --- a/src/mapi/glapi/gen/gl_XML.py
>> +++ b/src/mapi/glapi/gen/gl_XML.py
>> @@ -669,20 +669,25 @@ class gl_function( gl_item ):
>>          if exec_flavor:
>>              self.exec_flavor = exec_flavor
>>
>>          deprecated = element.get('deprecated', 'none')
>>          if deprecated != 'none':
>>              self.deprecated = Decimal(deprecated)
>>
>>          if not is_attr_true(element, 'desktop', 'true'):
>>              self.desktop = False
>>
>> +        if is_attr_true(element, 'no_error'):
>> +            self.has_no_error_variant = True
>> +        else:
>> +            self.has_no_error_variant = False
>> +
>>          if alias:
>>              true_name = alias
>>          else:
>>              true_name = name
>>
>>              # Only try to set the offset when a non-alias entry-point
>>              # is being processed.
>>
>>              if name in static_data.offsets:
>>                  self.offset = static_data.offsets[name]
>> diff --git a/src/mapi/glapi/gen/gl_genexec.py 
>> b/src/mapi/glapi/gen/gl_genexec.py
>> index 3a75419..ba82045 100644
>> --- a/src/mapi/glapi/gen/gl_genexec.py
>> +++ b/src/mapi/glapi/gen/gl_genexec.py
>> @@ -167,21 +167,21 @@ class PrintCode(gl_XML.gl_print_base):
>>
>>      def printRealHeader(self):
>>          print header
>>
>>      def printRealFooter(self):
>>          print footer
>>
>>      def printBody(self, api):
>>          # Collect SET_* calls by the condition under which they should
>>          # be called.
>> -        settings_by_condition = collections.defaultdict(lambda: [])
>> +        settings_by_condition = collections.defaultdict(lambda: 
>> collections.defaultdict(lambda: []))
>>          for f in api.functionIterateAll():
>>              if f.exec_flavor not in exec_flavor_map:
>>                  raise Exception(
>>                      'Unrecognized exec flavor 
>> {0!r}'.format(f.exec_flavor))
>>              condition_parts = []
>>              if f.name in apiexec.functions:
>>                  ex = apiexec.functions[f.name]
>>                  unconditional_count = 0
>>
>>                  if ex.compatibility is not None:
>> @@ -225,28 +225,37 @@ class PrintCode(gl_XML.gl_print_base):
>>
>>              if not condition_parts:
>>                  # This function does not exist in any API.
>>                  continue
>>              condition = ' || '.join(condition_parts)
>>              prefix = exec_flavor_map[f.exec_flavor]
>>              if prefix is None:
>>                  # This function is not implemented, or is dispatched
>>                  # dynamically.
>>                  continue
>> -            settings_by_condition[condition].append(
>> -                'SET_{0}(exec, {1}{0});'.format(f.name, prefix, f.name))
>> +            if f.has_no_error_variant:
>> +                
>> settings_by_condition[condition]['_mesa_is_no_error_enabled(ctx)'].append( 
>>
>> +                    'SET_{0}(exec, {1}{0}_no_error);'.format(f.name, 
>> prefix, f.name))
>> +                
>> settings_by_condition[condition]['!_mesa_is_no_error_enabled(ctx)'].append( 
>>
>> +                    'SET_{0}(exec, {1}{0});'.format(f.name, prefix, 
>> f.name))
>> +            else:
>> +                settings_by_condition[condition]['true'].append(
>> +                    'SET_{0}(exec, {1}{0});'.format(f.name, prefix, 
>> f.name))
>>          # Print out an if statement for each unique condition, with
>>          # the SET_* calls nested inside it.
>>          for condition in sorted(settings_by_condition.keys()):
>>              print '   if ({0}) {{'.format(condition)
>> -            for setting in sorted(settings_by_condition[condition]):
>> -                print '      {0}'.format(setting)
>> +            for khr_no_error in 
>> sorted(settings_by_condition[condition].keys()):
>> +                print '      if ({0}) {{'.format(khr_no_error)
>> +                for setting in 
>> sorted(settings_by_condition[condition][khr_no_error]):
>> +                    print '         {0}'.format(setting)
>> +                print '      }'
>>              print '   }'
> 
> I think this could be kept a bit simpler by saying
> 
>     no_error_condition = '({0}) && 
> _mesa_is_no_error_enabled(ctx)'.format(condition)
>     error_condition = '({0}) && 
> !_mesa_is_no_error_enabled(ctx)'.format(condition)
>     settings_by_condition[no_error_condition].append(...)
>     settings_by_condition[error_condition].append(...)
> 
> Though I don't feel too strongly about it.

The down side is that the variant setting calls end up separated in the 
generated file. This can make visual inspection of the file a little 
more difficult.

Anyway I've sent a version 2 with your suggestion. I'm not to worried 
either way.

> 
> Cheers,
> Nicolai
> 
>>
>>
>>  def _parser():
>>      """Parse arguments and return namespace."""
>>      parser = argparse.ArgumentParser()
>>      parser.add_argument('-f',
>>                          dest='filename',
>>                          default='gl_and_es_API.xml',
>>                          help='an xml file describing an API')
>>
> 
>