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

[Jason Ekstrand]

On Fri, Apr 22, 2016 at 10:27 AM, Emil Velikov <emil.l.velikov at gmail.com>
wrote:

> 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.
>

Yes, but if they run in parallel (which they will on *any* modern machine),
then it only takes as long as the longest one takes to run.  I don't think
it's actually helping anything to do them both with one python invocation
except for the make -j1 case.


> >>>>
> >>>> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160422/76d96307/attachment.html>