From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:59863) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMmxa-0005DW-3u for qemu-devel@nongnu.org; Fri, 12 Oct 2012 17:39:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TMmxY-0003Lc-Px for qemu-devel@nongnu.org; Fri, 12 Oct 2012 17:39:58 -0400 Received: from mail-ob0-f173.google.com ([209.85.214.173]:58427) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMmxY-0003LT-LB for qemu-devel@nongnu.org; Fri, 12 Oct 2012 17:39:56 -0400 Received: by mail-ob0-f173.google.com with SMTP id wc18so3149133obb.4 for ; Fri, 12 Oct 2012 14:39:56 -0700 (PDT) Sender: fluxion Date: Fri, 12 Oct 2012 16:39:52 -0500 From: Michael Roth Message-ID: <20121012213952.GR16157@illuin> References: <1349372021-31212-1-git-send-email-mdroth@linux.vnet.ibm.com> <1349372021-31212-23-git-send-email-mdroth@linux.vnet.ibm.com> <506E993E.2090001@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <506E993E.2090001@redhat.com> Subject: Re: [Qemu-devel] [PATCH v3 22/22] qidl: unit tests and build infrastructure List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: kwolf@redhat.com, peter.maydell@linaro.org, aliguori@us.ibm.com, qemu-devel@nongnu.org, blauwirbel@gmail.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 >