[Mesa-dev] [PATCH 20/25] anv: minor tweak in the generation script

[Emil Velikov]

On 22 April 2016 at 04:36, Kristian Høgsberg <krh at bitplanet.net> wrote:
> On Thu, Apr 21, 2016 at 5:18 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> On 21 April 2016 at 22:50, Jason Ekstrand <jason at jlekstrand.net> wrote:
>>> On Thu, Apr 21, 2016 at 6:16 AM, Emil Velikov <emil.l.velikov at gmail.com>
>>> wrote:
>>>>
>>>> From: Emil Velikov <emil.velikov at collabora.com>
>>>>
>>>> Rather than parsing through the same files (public headers) twice, tweak
>>>> the python script to create both files at the same time.
>>>
>>>
>>> Yes, but it takes almost zero time to generate them and it's going to run in
>>> parallel before anything else gets built.  I don't know that this really
>>> saves us anything.
>>>
>> Are you sure about this one? Based on my brief testing - things were
>> pretty much stalled until both files were generated. I'll take another
>> look.
>>
>> If anything the approach cuts down the bash output redirection and
>> some nasty handling around it.
>> Talking about the following $(PYHON) ....  > $@ || ($(RM) $@; false)
>> and how often we forget to add it.
>>
Things get stalled for all the sources to be generated before proceeding with
the compilation. Thus there isn't much benefit with the current approach afaict.

>>>>
>>>> Chances are that if the public headers change, both files will need to
>>>> be regenerated.
>>>>
>>>> Note to the python masters: this patch aims to be the least evasive
>>>> change. Feel free to change/rewrite things to your liking.
>>>>
>>>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>>>> ---
>>>>  src/intel/vulkan/Makefile.am            |  8 +++----
>>>>  src/intel/vulkan/anv_entrypoints_gen.py | 41
>>>> ++++++++++++++++++++++++---------
>>>>  2 files changed, 33 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/src/intel/vulkan/Makefile.am b/src/intel/vulkan/Makefile.am
>>>> index 360e97f..110961e 100644
>>>> --- a/src/intel/vulkan/Makefile.am
>>>> +++ b/src/intel/vulkan/Makefile.am
>>>> @@ -126,11 +126,9 @@ VULKAN_LIB_DEPS += \
>>>>
>>>>  libvulkan_intel_la_SOURCES = $(VULKAN_GEM_FILES)
>>>>
>>>> -anv_entrypoints.h : anv_entrypoints_gen.py $(vulkan_include_HEADERS)
>>>> -       $(AM_V_GEN) cat $(vulkan_include_HEADERS) | $(CPP) $(AM_CPPFLAGS)
>>>> - | $(PYTHON2) $< header > $@
>>>> -
>>>> -anv_entrypoints.c : anv_entrypoints_gen.py $(vulkan_include_HEADERS)
>>>> -       $(AM_V_GEN) cat $(vulkan_include_HEADERS) | $(CPP) $(AM_CPPFLAGS)
>>>> - | $(PYTHON2) $< code > $@
>>>> +anv_entrypoints.c anv_entrypoints.h : anv_entrypoints_gen.py
>>>> $(vulkan_include_HEADERS)
>>>> +       $(AM_V_GEN) cat $(vulkan_include_HEADERS) | $(CPP) $(AM_CPPFLAGS)
>>>> - | \
>>>> +       $(PYTHON2) $(srcdir)/anv_entrypoints_gen.py $(builddir)
>>>>
>>>>  CLEANFILES = $(BUILT_SOURCES)
>>>>
>>>> diff --git a/src/intel/vulkan/anv_entrypoints_gen.py
>>>> b/src/intel/vulkan/anv_entrypoints_gen.py
>>>> index cedecfe..19bfb93 100644
>>>> --- a/src/intel/vulkan/anv_entrypoints_gen.py
>>>> +++ b/src/intel/vulkan/anv_entrypoints_gen.py
>>>> @@ -22,7 +22,7 @@
>>>>  # IN THE SOFTWARE.
>>>>  #
>>>>
>>>> -import fileinput, re, sys
>>>> +import fileinput, re, sys, os
>>>>
>>>>  # Each function typedef in the vulkan.h header is all on one line and
>>>> matches
>>>>  # this regepx. We hope that won't change.
>>>> @@ -51,15 +51,21 @@ def hash(name):
>>>>
>>>>      return h
>>>>
>>>> -opt_header = False
>>>> -opt_code = False
>>>> +if len(sys.argv[1:]) != 1:
>>>> +    print "Usage: %s <output_directory>" % sys.argv[0]
>>>> +    exit(1)
>>>> +
>>>> +output_dir = sys.argv[1]
>>>> +if not os.path.isdir(output_dir):
>>>> +    if os.path.exists(output_dir):
>>>> +        print "ERROR: Invalid output directory: %s" % output_dir
>>>> +        exit(1)
>>>> +
>>>> +sys.argv.pop()
>>>> +# Output path exists, now just run the template
>>>> +output_file = os.sep.join([output_dir, 'anv_entrypoints.c'])
>>>> +output_header = os.sep.join([output_dir, 'anv_entrypoints.h'])
>>>>
>>>> -if (sys.argv[1] == "header"):
>>>> -    opt_header = True
>>>> -    sys.argv.pop()
>>>> -elif (sys.argv[1] == "code"):
>>>> -    opt_code = True
>>>> -    sys.argv.pop()
>>>>
>>>>  # Parse the entry points in the header
>>>>
>>>> @@ -77,7 +83,11 @@ for line in fileinput.input():
>>>>  # For outputting entrypoints.h we generate a anv_EntryPoint() prototype
>>>>  # per entry point.
>>>>
>>>> -if opt_header:
>>>> +def generate_header(output_header):
>>>> +    orig_stdout = sys.stdout
>>>> +    hdr = file(output_header, 'w')
>>>> +    sys.stdout = hdr
>>>
>>>
>>> This is a bit ugly...
>>>
>> s/a bit/a fair bit/
>>
>> Definitely, but in all honesty the currently the script isn't the
>> prettiest one either. I'm pretty sure Dylan will end up rewriting the
>> whole thing if he has the time/chance. Even without this patch ;-)
>
> The rewrite that would be worthwhile is to make it parse the spec xml.
> Not sure churning this script really helps anything.
>
Precisely why I've went ahead with the fastest/least evasive solution :-)

-Emil