[70/81] remove last 3 uses of :=, everywhere else uses += or =
diff mbox

Message ID fb6652759c6a5f980d8abe72f5bbeb58f160fd10.1249301360.git.quintela@redhat.com
State Superseded
Headers show

Commit Message

Juan Quintela Aug. 3, 2009, 12:47 p.m. UTC
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 Makefile.target |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

Comments

Hollis Blanchard Aug. 26, 2009, 9:10 p.m. UTC | #1
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
Juan Quintela Aug. 26, 2009, 9:46 p.m. UTC | #2
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.
Hollis Blanchard Aug. 26, 2009, 10:15 p.m. UTC | #3
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
Juan Quintela Aug. 26, 2009, 10:26 p.m. UTC | #4
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.

Patch
diff mbox

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)