Re: [Qemu-devel] [PATCH v3 22/22] qidl: unit tests and build infrastructure

[Michael Roth <mdroth@linux.vnet.ibm.com>]

On Fri, Oct 05, 2012 at 10:24:30AM +0200, Paolo Bonzini wrote:
> Il 04/10/2012 19:33, Michael Roth ha scritto:
> > +
> > +%.qidl.c: %.c $(SRC_PATH)/qidl.h $(addprefix $(SRC_PATH)/scripts/,lexer.py qidl.py qidl_parser.py qapi.py qapi_visit.py)
> > +	$(call rm -f $(*D)/qidl-generated/$(*F).qidl.c)
> > +	$(if $(strip $(shell grep "QIDL_ENABLE()" $< 1>/dev/null && echo "true")), \
> > +	  $(call quiet-command, \
> > +	    $(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(CFLAGS) -E -c -DQIDL_GEN $< | \
> > +	    $(PYTHON) $(SRC_PATH)/scripts/qidl.py \
> > +	    --output-filepath=$(*D)/qidl-generated/$(*F).qidl.c || [ "$$?" -eq 2 ], \
> > +	    "qidl PP $(*D)/$(*F).c"),)
> > +%.o: %.c %.qidl.c
> > +	$(if $(strip $(shell test -f $(*D)/qidl-generated/$(*F).qidl.c && echo "true")), \
> > +	  $(call quiet-command, \
> > +	    $(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c \
> > +		-DQIDL_ENABLED -include $< -o $@ $(*D)/qidl-generated/$(*F).qidl.c, \
> > +		"qidl CC $@"), \
> > +	  $(call quiet-command, \
> > +	    $(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c \
> > +	      -o $@ $<,"  CC    $@"))
> 
> 
> Because the .qidl.c files are not created for files without a QIDL_ENABLE()
> directive, all those files will be grepped on every invocation of the makefile.
> 
> It is better to define the list of QIDL_ENABLEd files in an auxiliary makefile
> like this (untested):

Thanks for the suggestion / example.

FYI, I tried to get this working but didn't manage to get it in for v4. Everything
seems to be falling back to the non-qidl %o: target, and I haven't had time to
debug it.

FWIW, it doesn't seem to be a major factor performance-wise. Current build
times on my laptop are (for all-target builds):

QIDL v4:
 real    8m35.383s
 user    31m1.844s
 sys     1m33.998s

upstream:
 real    8m28.181s
 user    30m44.983s
 sys     1m29.926s

There's also a potential performance trade-off with the suggested approach in that
a smaller build might be slower due to us always grepping over an exhaustive list
of possibly QIDL-fied files. To be optimal we'd want to limit the search to objects
the target actually pulls in.

Maintainance-wise, there's also a nice benefit in the current approach in that
we don't need to maintain a list of files to scan through, so anything we add to
the tree will get picked up by QIDL automatically.

So hopefully the current approach is a reasonable start for now.

> 
> QIDL_FILES =
> -include qemu-idl-files.mak
> qemu-idl-files.mak: $(obj-y) $(common-obj-y) $(hw-obj-y)
> 	(grep -l "QIDL_ENABLE()" $^ || :) | sed 's/^/QIDL_FILES += /' > $@
> 
> $(QIDL_FILES): %.qidl.c: %.c
> 	$(call quiet-command, \
> 	    $(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(CFLAGS) -E -c -DQIDL_GEN $< | \
> 	    $(PYTHON) $(SRC_PATH)/scripts/qidl.py \
> 	    --output-filepath=$(*D)/qidl-generated/tmp-$(*F).qidl.c || [ "$$?" -eq 2 ], \
> 	    "qidl PP $(*D)/$(*F).c"),)
> 	mv $(*D)/qidl-generated/tmp-$(*F).qidl.c $(*D)/qidl-generated/$(*F).qidl.c
> 
> $(QIDL_FILES): %.o: %.c %.qidl.c
> 	$(call quiet-command, \
> 	    $(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c \
> 		-DQIDL_ENABLED -include $< -o $@ $(*D)/qidl-generated/$(*F).qidl.c, \
> 		"qidl CC $@"), \
> 
> %.o: %.c
> 	$(call quiet-command, \
> 	    $(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c \
> 	      -o $@ $<,"  CC    $@"))
> 
> Paolo
>