[dpdk-dev] [PATCH] mk: fix link with gcc

[dpdk-dev] [PATCH] mk: fix link with gcc

[Thomas Monjalon]

Some linker options were not prefixed by -Wl, when using gcc:
	-z muldefs
	-melf_i386 (32-bit config)

Using macro linkerprefix is fixing it.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 mk/rte.lib.mk | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
index f5d2789..c58e68e 100644
--- a/mk/rte.lib.mk
+++ b/mk/rte.lib.mk
@@ -62,6 +62,8 @@ exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1))))
 ifeq ($(LINK_USING_CC),1)
 # Override the definition of LD here, since we're linking with CC
 LD := $(CC)
+LD_MULDEFS := $(call linkerprefix,-z$(comma)muldefs)
+CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS))
 endif
 
 O_TO_A = $(AR) crus $(LIB) $(OBJS-y)
@@ -73,7 +75,7 @@ O_TO_A_DO = @set -e; \
 	$(O_TO_A) && \
 	echo $(O_TO_A_CMD) > $(call exe2cmd,$(@))
 
-O_TO_S = $(LD) $(CPU_LDFLAGS) -z muldefs -shared $(OBJS-y) -o $(LIB)
+O_TO_S = $(LD) $(CPU_LDFLAGS) $(LD_MULDEFS) -shared $(OBJS-y) -o $(LIB)
 O_TO_S_STR = $(subst ','\'',$(O_TO_S)) #'# fix syntax highlight
 O_TO_S_DISP = $(if $(V),"$(O_TO_S_STR)","  LD $(@)")
 O_TO_S_DO = @set -e; \
@@ -89,7 +91,7 @@ O_TO_C_DO = @set -e; \
 	$(lib_dir) \
 	$(copy_obj)
 else
-O_TO_C = $(LD) -z muldefs -shared $(OBJS-y) -o $(LIB_ONE)
+O_TO_C = $(LD) $(LD_MULDEFS) -shared $(OBJS-y) -o $(LIB_ONE)
 O_TO_C_STR = $(subst ','\'',$(O_TO_C)) #'# fix syntax highlight
 O_TO_C_DISP = $(if $(V),"$(O_TO_C_STR)","  LD_C $(@)")
 O_TO_C_DO = @set -e; \
-- 
1.9.2

Re: [dpdk-dev] [PATCH] mk: fix link with gcc

[Olivier MATZ]

Hi Thomas,

On 05/27/2014 02:55 PM, Thomas Monjalon wrote:
> Some linker options were not prefixed by -Wl, when using gcc:
> 	-z muldefs
> 	-melf_i386 (32-bit config)
>
> Using macro linkerprefix is fixing it.
>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>

The patch looks correct, but from the commit log it's difficult
to understand what is the problem today. Is there a compilation
issue? Or is it just cleaning?

Regards,
Olivier

Re: [dpdk-dev] [PATCH] mk: fix link with gcc

[Neil Horman]

On Tue, May 27, 2014 at 02:55:16PM +0200, Thomas Monjalon wrote:
> Some linker options were not prefixed by -Wl, when using gcc:
> 	-z muldefs
> 	-melf_i386 (32-bit config)
> 
> Using macro linkerprefix is fixing it.
> 
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> ---
>  mk/rte.lib.mk | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
> index f5d2789..c58e68e 100644
> --- a/mk/rte.lib.mk
> +++ b/mk/rte.lib.mk
> @@ -62,6 +62,8 @@ exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1))))
>  ifeq ($(LINK_USING_CC),1)
>  # Override the definition of LD here, since we're linking with CC
>  LD := $(CC)
> +LD_MULDEFS := $(call linkerprefix,-z$(comma)muldefs)
> +CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS))
>  endif
>  
Agree with Olivier, what exactly is the problem here?  Also, I don't think this
is correct, as CPU_LD_FLAGS and -z muldefs below is used in conjunction with
$LD.  It would make sense to prefix -Wl to these options if we were passing them
through $CC, but not $LD

Neil

>  O_TO_A = $(AR) crus $(LIB) $(OBJS-y)
> @@ -73,7 +75,7 @@ O_TO_A_DO = @set -e; \
>  	$(O_TO_A) && \
>  	echo $(O_TO_A_CMD) > $(call exe2cmd,$(@))
>  
> -O_TO_S = $(LD) $(CPU_LDFLAGS) -z muldefs -shared $(OBJS-y) -o $(LIB)
> +O_TO_S = $(LD) $(CPU_LDFLAGS) $(LD_MULDEFS) -shared $(OBJS-y) -o $(LIB)
>  O_TO_S_STR = $(subst ','\'',$(O_TO_S)) #'# fix syntax highlight
>  O_TO_S_DISP = $(if $(V),"$(O_TO_S_STR)","  LD $(@)")
>  O_TO_S_DO = @set -e; \
> @@ -89,7 +91,7 @@ O_TO_C_DO = @set -e; \
>  	$(lib_dir) \
>  	$(copy_obj)
>  else
> -O_TO_C = $(LD) -z muldefs -shared $(OBJS-y) -o $(LIB_ONE)
> +O_TO_C = $(LD) $(LD_MULDEFS) -shared $(OBJS-y) -o $(LIB_ONE)
>  O_TO_C_STR = $(subst ','\'',$(O_TO_C)) #'# fix syntax highlight
>  O_TO_C_DISP = $(if $(V),"$(O_TO_C_STR)","  LD_C $(@)")
>  O_TO_C_DO = @set -e; \
> -- 
> 1.9.2
> 
> 

Re: [dpdk-dev] [PATCH] mk: fix link with gcc

[Thomas Monjalon]

Hi Neil,

2014-05-28 10:17, Neil Horman:
> On Tue, May 27, 2014 at 02:55:16PM +0200, Thomas Monjalon wrote:
> > Some linker options were not prefixed by -Wl, when using gcc:
> > 	-z muldefs
> > 	-melf_i386 (32-bit config)
> > 
> > Using macro linkerprefix is fixing it.
> > 
> > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
[...]
> >  ifeq ($(LINK_USING_CC),1)
> >  # Override the definition of LD here, since we're linking with CC
> >  LD := $(CC)
> > 
> > +LD_MULDEFS := $(call linkerprefix,-z$(comma)muldefs)
> > +CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS))
> 
> Agree with Olivier, what exactly is the problem here?

When using CC as LD, linker options should be prefixed with -Wl.

> Also, I don't think
> this is correct, as CPU_LD_FLAGS and -z muldefs below is used in
> conjunction with $LD.  It would make sense to prefix -Wl to these options
> if we were passing them through $CC, but not $LD

Yes, but options are prefixed only in the case LD = CC.

Neil, this situation is funny as you're the author of the patch making LD as 
CC and you submitted this kind of fix to prefix CPU_LDFLAGS :)
This patch is a translation of yours with use of macro linkerprefix.

--  
Thomas

Re: [dpdk-dev] [PATCH] mk: fix link with gcc

[Thomas Monjalon]

Hi Olivier,

2014-05-28 13:47, Olivier MATZ:
> On 05/27/2014 02:55 PM, Thomas Monjalon wrote:
> > Some linker options were not prefixed by -Wl, when using gcc:
> > 	-z muldefs
> > 	-melf_i386 (32-bit config)
> > 
> > Using macro linkerprefix is fixing it.
> > 
> > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> 
> The patch looks correct, but from the commit log it's difficult
> to understand what is the problem today. Is there a compilation
> issue? Or is it just cleaning?

You're right, title should be:
	mk: fix 32-bit link with gcc

And I should add some details in the commit log:

I didn't see any error with -z muldefs but it isn't documented in gcc manual. 
So it's safer to explicitly pass it to the linker.

The variable CPU_LDFLAGS contains "-melf_i386" in 32-bit configurations. So 
building 32-bit shared library raises this error:
	gcc: error: unrecognized command line option ‘-melf_i386’

Olivier, I'll make these changes if you (or Neil) ack the patch.

Thanks for review
-- 
Thomas

Re: [dpdk-dev] [PATCH] mk: fix link with gcc

[Neil Horman]

On Thu, May 29, 2014 at 08:24:56AM +0200, Thomas Monjalon wrote:
> Hi Neil,
> 
> 2014-05-28 10:17, Neil Horman:
> > On Tue, May 27, 2014 at 02:55:16PM +0200, Thomas Monjalon wrote:
> > > Some linker options were not prefixed by -Wl, when using gcc:
> > > 	-z muldefs
> > > 	-melf_i386 (32-bit config)
> > > 
> > > Using macro linkerprefix is fixing it.
> > > 
> > > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> [...]
> > >  ifeq ($(LINK_USING_CC),1)
> > >  # Override the definition of LD here, since we're linking with CC
> > >  LD := $(CC)
> > > 
> > > +LD_MULDEFS := $(call linkerprefix,-z$(comma)muldefs)
> > > +CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS))
> > 
> > Agree with Olivier, what exactly is the problem here?
> 
> When using CC as LD, linker options should be prefixed with -Wl.
> 
> > Also, I don't think
> > this is correct, as CPU_LD_FLAGS and -z muldefs below is used in
> > conjunction with $LD.  It would make sense to prefix -Wl to these options
> > if we were passing them through $CC, but not $LD
> 
> Yes, but options are prefixed only in the case LD = CC.
> 
ah, sorry, was looking at the wrong clause in rte.lib.mk

> Neil, this situation is funny as you're the author of the patch making LD as 
> CC and you submitted this kind of fix to prefix CPU_LDFLAGS :)
> This patch is a translation of yours with use of macro linkerprefix.
> 
Yeah, my fault, I was only looking at the lines changed, rather than the context
surrounding them.  This makes sense.
Neil

> --  
> Thomas
> 

Re: [dpdk-dev] [PATCH] mk: fix link with gcc

[Olivier MATZ]

Hi Thomas,

On 05/29/2014 08:48 AM, Thomas Monjalon wrote:
> You're right, title should be:
> 	mk: fix 32-bit link with gcc
>
> And I should add some details in the commit log:
>
> I didn't see any error with -z muldefs but it isn't documented in gcc manual.
> So it's safer to explicitly pass it to the linker.
>
> The variable CPU_LDFLAGS contains "-melf_i386" in 32-bit configurations. So
> building 32-bit shared library raises this error:
> 	gcc: error: unrecognized command line option ‘-melf_i386’
>
> Olivier, I'll make these changes if you (or Neil) ack the patch.

Acked-by: Olivier Matz <olivier.matz@6wind.com>

Re: [dpdk-dev] [PATCH] mk: fix link with gcc

[Thomas Monjalon]

2014-06-02 09:40, Olivier MATZ:
> Hi Thomas,
> 
> On 05/29/2014 08:48 AM, Thomas Monjalon wrote:
> > You're right, title should be:
> > 	mk: fix 32-bit link with gcc
> >
> > And I should add some details in the commit log:
> >
> > I didn't see any error with -z muldefs but it isn't documented in gcc manual.
> > So it's safer to explicitly pass it to the linker.
> >
> > The variable CPU_LDFLAGS contains "-melf_i386" in 32-bit configurations. So
> > building 32-bit shared library raises this error:
> > 	gcc: error: unrecognized command line option ‘-melf_i386’
> >
> > Olivier, I'll make these changes if you (or Neil) ack the patch.
> 
> Acked-by: Olivier Matz <olivier.matz@6wind.com>

Applied for version 1.7.0.

-- 
Thomas