| Message ID | fb6652759c6a5f980d8abe72f5bbeb58f160fd10.1249301360.git.quintela@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, Aug 3, 2009 at 5:47 AM, Juan Quintela<quintela@redhat.com> wrote: > > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > Makefile.target | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/Makefile.target b/Makefile.target > index a4d269d..6b3d40f 100644 > --- a/Makefile.target > +++ b/Makefile.target > @@ -20,7 +20,7 @@ endif > PROGS=$(QEMU_PROG) > > ifeq ($(subst ppc64,ppc,$(ARCH))$(TARGET_BASE_ARCH),ppcppc) > -translate.o: QEMU_CFLAGS := $(QEMU_CFLAGS) $(call cc-option, $(QEMU_CFLAGS), -fno-unit-at-a-time,) > +translate.o: QEMU_CFLAGS += $(call cc-option, $(QEMU_CFLAGS), -fno-unit-at-a-time,) > endif > > LIBS+=-lm This change breaks the build with PPC host and PPC target: Makefile:25: *** Recursive variable `QEMU_CFLAGS' references itself (eventually). Stop. I don't know why you want to remove all :=, but it seems like this hunk should be reverted. -Hollis
Hollis Blanchard <hollis@penguinppc.org> wrote: > On Mon, Aug 3, 2009 at 5:47 AM, Juan Quintela<quintela@redhat.com> wrote: >> >> Signed-off-by: Juan Quintela <quintela@redhat.com> >> --- >> Makefile.target | 6 +++--- >> 1 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/Makefile.target b/Makefile.target >> index a4d269d..6b3d40f 100644 >> --- a/Makefile.target >> +++ b/Makefile.target >> @@ -20,7 +20,7 @@ endif >> PROGS=$(QEMU_PROG) >> >> ifeq ($(subst ppc64,ppc,$(ARCH))$(TARGET_BASE_ARCH),ppcppc) >> -translate.o: QEMU_CFLAGS := $(QEMU_CFLAGS) $(call cc-option, $(QEMU_CFLAGS), -fno-unit-at-a-time,) >> +translate.o: QEMU_CFLAGS += $(call cc-option, $(QEMU_CFLAGS), -fno-unit-at-a-time,) >> endif >> >> LIBS+=-lm > > This change breaks the build with PPC host and PPC target: > Makefile:25: *** Recursive variable `QEMU_CFLAGS' references itself > (eventually). Stop. > > I don't know why you want to remove all :=, but it seems like this > hunk should be reverted. Could you check two things: a- remove the whole block, it should not be needed anymore (or that I have been told) b- if a) don't work, tryng to change the interior bit to translate.o: QEMU_CFLAGS += $(call cc-option, $(CFLAGS),-fno-unit-at-a-time,) And if so, we can do this change. What we pass as 2nd argument shouldn't matter for this test. call cc-option is complicated to get right, sorry :( Later, Juan.
On Wed, Aug 26, 2009 at 2:46 PM, Juan Quintela<quintela@redhat.com> wrote: > Hollis Blanchard <hollis@penguinppc.org> wrote: >> On Mon, Aug 3, 2009 at 5:47 AM, Juan Quintela<quintela@redhat.com> wrote: >>> >>> Signed-off-by: Juan Quintela <quintela@redhat.com> >>> --- >>> Makefile.target | 6 +++--- >>> 1 files changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/Makefile.target b/Makefile.target >>> index a4d269d..6b3d40f 100644 >>> --- a/Makefile.target >>> +++ b/Makefile.target >>> @@ -20,7 +20,7 @@ endif >>> PROGS=$(QEMU_PROG) >>> >>> ifeq ($(subst ppc64,ppc,$(ARCH))$(TARGET_BASE_ARCH),ppcppc) >>> -translate.o: QEMU_CFLAGS := $(QEMU_CFLAGS) $(call cc-option, $(QEMU_CFLAGS), -fno-unit-at-a-time,) >>> +translate.o: QEMU_CFLAGS += $(call cc-option, $(QEMU_CFLAGS), -fno-unit-at-a-time,) >>> endif >>> >>> LIBS+=-lm >> >> This change breaks the build with PPC host and PPC target: >> Makefile:25: *** Recursive variable `QEMU_CFLAGS' references itself >> (eventually). Stop. >> >> I don't know why you want to remove all :=, but it seems like this >> hunk should be reverted. > > Could you check two things: > > a- remove the whole block, it should not be needed anymore (or that I > have been told) The original commit, from malc, says it's necessary to work around a gcc 4.3.0 bug. If that's true, it's probably not a great idea to remove it. > b- if a) don't work, tryng to change the interior bit to > > translate.o: QEMU_CFLAGS += $(call cc-option, $(CFLAGS),-fno-unit-at-a-time,) > > And if so, we can do this change. What we pass as 2nd argument > shouldn't matter for this test. > > call cc-option is complicated to get right, sorry :( If it doesn't matter, better not to pass anything, rather than confuse the reader into thinking it does matter. This seems to work: translate.o: QEMU_CFLAGS += $(call cc-option, , -fno-unit-at-a-time,) But strangely, this doesn't: translate.o: QEMU_CFLAGS += $(call cc-option, "", -fno-unit-at-a-time,) However, what if the second argument did matter? I don't understand the problem with the original syntax. Debugging makefiles sucks. -Hollis
Hollis Blanchard <hollis@penguinppc.org> wrote: > On Wed, Aug 26, 2009 at 2:46 PM, Juan Quintela<quintela@redhat.com> wrote: >> Hollis Blanchard <hollis@penguinppc.org> wrote: >>> On Mon, Aug 3, 2009 at 5:47 AM, Juan Quintela<quintela@redhat.com> wrote: >> Could you check two things: >> >> a- remove the whole block, it should not be needed anymore (or that I >> have been told) > > The original commit, from malc, says it's necessary to work around a > gcc 4.3.0 bug. If that's true, it's probably not a great idea to > remove it. From malc: -fno-unit-at-a-time (-fno-toplevel-reorder which -fno-unit-at-a-time implies actually) is no longer needed after BlueSwirls work on PPC's translate. it appears that they changed the file that was misscompiled (I don't have more information). I was waiting for someone with ppc access to test its removal (only ifeq remaining on Makefile.target that depends of architecture, all other are gone (but I was not able to test this one, hint, hint, :) >> b- if a) don't work, tryng to change the interior bit to >> >> translate.o: QEMU_CFLAGS += $(call cc-option, $(CFLAGS),-fno-unit-at-a-time,) >> >> And if so, we can do this change. What we pass as 2nd argument >> shouldn't matter for this test. >> >> call cc-option is complicated to get right, sorry :( > > If it doesn't matter, better not to pass anything, rather than confuse > the reader into thinking it does matter. This seems to work: > translate.o: QEMU_CFLAGS += $(call cc-option, , -fno-unit-at-a-time,) > > But strangely, this doesn't: > translate.o: QEMU_CFLAGS += $(call cc-option, "", -fno-unit-at-a-time,) I told you that call cc-option was bad :) Look at its definition: # cc-option # Usage: CFLAGS+=$(call cc-option, $(CFLAGS), -falign-functions=0, -malign-functions=0) cc-option = $(shell if $(CC) $(1) $(2) -S -o /dev/null -xc /dev/null \ > /dev/null 2>&1; then echo "$(2)"; else echo "$(3)"; fi ;) it appears that gcc don't like an empty argument in the middle :p Actually, I think that example and macro is just wrong. 2nd argument shouldn't exist at all. We are testing if compiler accept some option, shouldn't be needed any $CFLAGS at all. But I didn't want to change so many things. > However, what if the second argument did matter? I don't understand > the problem with the original syntax. There were (at least) 3 different styles on Makefiles, now there is only one. We had +=, := and I think some other funny variants. Now we only have += Later, Juan.
diff --git a/Makefile.target b/Makefile.target index a4d269d..6b3d40f 100644 --- a/Makefile.target +++ b/Makefile.target @@ -20,7 +20,7 @@ endif PROGS=$(QEMU_PROG) ifeq ($(subst ppc64,ppc,$(ARCH))$(TARGET_BASE_ARCH),ppcppc) -translate.o: QEMU_CFLAGS := $(QEMU_CFLAGS) $(call cc-option, $(QEMU_CFLAGS), -fno-unit-at-a-time,) +translate.o: QEMU_CFLAGS += $(call cc-option, $(QEMU_CFLAGS), -fno-unit-at-a-time,) endif LIBS+=-lm @@ -107,7 +107,7 @@ ifeq ($(TARGET_ARCH), i386) obj-y += vm86.o endif -nwfpe-obj-y := fpa11.o fpa11_cpdo.o fpa11_cpdt.o fpa11_cprt.o fpopcode.o +nwfpe-obj-y = fpa11.o fpa11_cpdo.o fpa11_cpdt.o fpa11_cprt.o fpopcode.o nwfpe-obj-y += single_cpdo.o double_cpdo.o extended_cpdo.o obj-arm-y += $(addprefix nwfpe/, $(nwfpe-obj-y)) obj-arm-y += arm-semi.o @@ -174,7 +174,7 @@ sound-obj-$(CONFIG_ADLIB) += fmopl.o adlib.o sound-obj-$(CONFIG_GUS) += gus.o gusemu_hal.o gusemu_mixer.o sound-obj-$(CONFIG_CS4231A) += cs4231a.o -adlib.o fmopl.o: QEMU_CFLAGS := ${QEMU_CFLAGS} -DBUILD_Y8950=0 +adlib.o fmopl.o: QEMU_CFLAGS += -DBUILD_Y8950=0 QEMU_CFLAGS += $(VNC_TLS_CFLAGS) QEMU_CFLAGS += $(VNC_SASL_CFLAGS)
Signed-off-by: Juan Quintela <quintela@redhat.com> --- Makefile.target | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)