DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] mk: link combined lib using CC
@ 2014-10-23 15:35 Sergio Gonzalez Monroy
  2014-10-28 11:45 ` De Lara Guarch, Pablo
  2014-10-28 15:48 ` [dpdk-dev] [PATCH v2] mk: link combined shared " Sergio Gonzalez Monroy
  0 siblings, 2 replies; 17+ messages in thread
From: Sergio Gonzalez Monroy @ 2014-10-23 15:35 UTC (permalink / raw)
  To: dev

Building combined shared libs fails if we set EXTRA_CFLAGS=-O0.

/usr/bin/ld: test: hidden symbol `mknod' in /usr/lib64/libc_nonshared.a(mknod.oS) is referenced by DSO
/usr/bin/ld: final link failed: Bad value
collect2: error: ld returned 1 exit status

Fix: link combined shared lib using CC if LINK_USING_CC is enabled.

Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
---
 mk/rte.lib.mk      |  1 -
 mk/rte.sharelib.mk | 12 +++++++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
index d83e808..a6abd6d 100644
--- a/mk/rte.lib.mk
+++ b/mk/rte.lib.mk
@@ -63,7 +63,6 @@ ifeq ($(LINK_USING_CC),1)
 # Override the definition of LD here, since we're linking with CC
 LD := $(CC) $(CPU_CFLAGS)
 LD_MULDEFS := $(call linkerprefix,-z$(comma)muldefs)
-CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS))
 endif
 
 O_TO_A = $(AR) crus $(LIB) $(OBJS-y)
diff --git a/mk/rte.sharelib.mk b/mk/rte.sharelib.mk
index c0a811a..1fac0ad 100644
--- a/mk/rte.sharelib.mk
+++ b/mk/rte.sharelib.mk
@@ -29,6 +29,8 @@
 #   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 #   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+include $(RTE_SDK)/mk/internal/rte.build-pre.mk
+
 # VPATH contains at least SRCDIR
 VPATH += $(SRCDIR)
 
@@ -45,7 +47,15 @@ sharelib: $(LIB_ONE) FORCE
 
 OBJS = $(wildcard $(RTE_OUTPUT)/build/lib/*.o)
 
-O_TO_S = $(LD) $(CPU_LDFLAGS) -shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE)
+ifeq ($(LINK_USING_CC),1)
+# Override the definition of LD here, since we're linking with CC
+LD := $(CC) $(CPU_CFLAGS)
+LD_MULDEFS := $(call linkerprefix,-z$(comma)muldefs)
+CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS))
+endif
+
+O_TO_S = $(LD) $(CPU_LDFLAGS) $(LD_MULDEFS) -shared $(OBJS)\
+	-o $(RTE_OUTPUT)/lib/$(LIB_ONE)
 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_CMD = "cmd_$@ = $(O_TO_S_STR)"
-- 
1.9.3

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH] mk: link combined lib using CC
  2014-10-23 15:35 [dpdk-dev] [PATCH] mk: link combined lib using CC Sergio Gonzalez Monroy
@ 2014-10-28 11:45 ` De Lara Guarch, Pablo
  2014-10-28 14:51   ` Sergio Gonzalez Monroy
  2014-10-28 15:48 ` [dpdk-dev] [PATCH v2] mk: link combined shared " Sergio Gonzalez Monroy
  1 sibling, 1 reply; 17+ messages in thread
From: De Lara Guarch, Pablo @ 2014-10-28 11:45 UTC (permalink / raw)
  To: Gonzalez Monroy, Sergio, dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Sergio Gonzalez
> Monroy
> Sent: Thursday, October 23, 2014 4:36 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] mk: link combined lib using CC
> 
> Building combined shared libs fails if we set EXTRA_CFLAGS=-O0.
> 
> /usr/bin/ld: test: hidden symbol `mknod' in
> /usr/lib64/libc_nonshared.a(mknod.oS) is referenced by DSO
> /usr/bin/ld: final link failed: Bad value
> collect2: error: ld returned 1 exit status
> 
> Fix: link combined shared lib using CC if LINK_USING_CC is enabled.
> 
> Signed-off-by: Sergio Gonzalez Monroy
> <sergio.gonzalez.monroy@intel.com>
> ---
>  mk/rte.lib.mk      |  1 -
>  mk/rte.sharelib.mk | 12 +++++++++++-
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
> index d83e808..a6abd6d 100644
> --- a/mk/rte.lib.mk
> +++ b/mk/rte.lib.mk
> @@ -63,7 +63,6 @@ ifeq ($(LINK_USING_CC),1)
>  # Override the definition of LD here, since we're linking with CC
>  LD := $(CC) $(CPU_CFLAGS)
>  LD_MULDEFS := $(call linkerprefix,-z$(comma)muldefs)
> -CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS))
>  endif

Patch does not apply cleanly, due to context mismatch. 
Could you send a V2, based on the current branch status?

Plus, should we include compilation errors in commits? 
They are quite useful to identify the problem that 
the patch is solving, but not sure if this should be shown in the git log.

Thanks,
Pablo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH] mk: link combined lib using CC
  2014-10-28 11:45 ` De Lara Guarch, Pablo
@ 2014-10-28 14:51   ` Sergio Gonzalez Monroy
  2014-10-28 15:33     ` Thomas Monjalon
  0 siblings, 1 reply; 17+ messages in thread
From: Sergio Gonzalez Monroy @ 2014-10-28 14:51 UTC (permalink / raw)
  To: De Lara Guarch, Pablo; +Cc: dev

On Tue, Oct 28, 2014 at 11:45:28AM +0000, De Lara Guarch, Pablo wrote:
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Sergio Gonzalez
> > Monroy
> > Sent: Thursday, October 23, 2014 4:36 PM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH] mk: link combined lib using CC
> > 
> > Building combined shared libs fails if we set EXTRA_CFLAGS=-O0.
> > 
> > /usr/bin/ld: test: hidden symbol `mknod' in
> > /usr/lib64/libc_nonshared.a(mknod.oS) is referenced by DSO
> > /usr/bin/ld: final link failed: Bad value
> > collect2: error: ld returned 1 exit status
> > 
> > Fix: link combined shared lib using CC if LINK_USING_CC is enabled.
> > 
> > Signed-off-by: Sergio Gonzalez Monroy
> > <sergio.gonzalez.monroy@intel.com>
> 
> Plus, should we include compilation errors in commits? 
> They are quite useful to identify the problem that 
> the patch is solving, but not sure if this should be shown in the git log.
> 
I was wondering about it myself. I think it is useful info but maybe t is more
appropiate to have it as a comment or cover letter just on the mailing list.
I don't have a strong preference for including it, maybe someone else has an
opinion about this?

Thanks,
Sergio

> Thanks,
> Pablo
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH] mk: link combined lib using CC
  2014-10-28 14:51   ` Sergio Gonzalez Monroy
@ 2014-10-28 15:33     ` Thomas Monjalon
  2014-10-28 15:39       ` Sergio Gonzalez Monroy
  0 siblings, 1 reply; 17+ messages in thread
From: Thomas Monjalon @ 2014-10-28 15:33 UTC (permalink / raw)
  To: Sergio Gonzalez Monroy; +Cc: dev

2014-10-28 14:51, Sergio Gonzalez Monroy:
> On Tue, Oct 28, 2014 at 11:45:28AM +0000, De Lara Guarch, Pablo wrote:
> > > Building combined shared libs fails if we set EXTRA_CFLAGS=-O0.
> > > 
> > > /usr/bin/ld: test: hidden symbol `mknod' in
> > > /usr/lib64/libc_nonshared.a(mknod.oS) is referenced by DSO
> > > /usr/bin/ld: final link failed: Bad value
> > > collect2: error: ld returned 1 exit status
> > > 
> > > Fix: link combined shared lib using CC if LINK_USING_CC is enabled.
> > > 
> > > Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> > 
> > Plus, should we include compilation errors in commits? 
> > They are quite useful to identify the problem that 
> > the patch is solving, but not sure if this should be shown in the git log.
> > 
> I was wondering about it myself. I think it is useful info but maybe t is more
> appropiate to have it as a comment or cover letter just on the mailing list.
> I don't have a strong preference for including it, maybe someone else has an
> opinion about this?

We are not limited in characters in the commit log. So every useful information
(like error output) is more than welcome.
The only thing which needs to be shorter than a twitter post, is the title.
Because short and clear titles help to scroll commits.

That said, don't put things which have no interest at all. Here I'd put only this:
	ld: test: hidden symbol `mknod' in /usr/lib64/libc_nonshared.a(mknod.oS) is referenced by DSO

-- 
Thomas

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH] mk: link combined lib using CC
  2014-10-28 15:33     ` Thomas Monjalon
@ 2014-10-28 15:39       ` Sergio Gonzalez Monroy
  0 siblings, 0 replies; 17+ messages in thread
From: Sergio Gonzalez Monroy @ 2014-10-28 15:39 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Tue, Oct 28, 2014 at 04:33:18PM +0100, Thomas Monjalon wrote:
> 2014-10-28 14:51, Sergio Gonzalez Monroy:
> > On Tue, Oct 28, 2014 at 11:45:28AM +0000, De Lara Guarch, Pablo wrote:
> > > > Building combined shared libs fails if we set EXTRA_CFLAGS=-O0.
> > > > 
> > > > /usr/bin/ld: test: hidden symbol `mknod' in
> > > > /usr/lib64/libc_nonshared.a(mknod.oS) is referenced by DSO
> > > > /usr/bin/ld: final link failed: Bad value
> > > > collect2: error: ld returned 1 exit status
> > > > 
> > > > Fix: link combined shared lib using CC if LINK_USING_CC is enabled.
> > > > 
> > > > Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> > > 
> > > Plus, should we include compilation errors in commits? 
> > > They are quite useful to identify the problem that 
> > > the patch is solving, but not sure if this should be shown in the git log.
> > > 
> > I was wondering about it myself. I think it is useful info but maybe t is more
> > appropiate to have it as a comment or cover letter just on the mailing list.
> > I don't have a strong preference for including it, maybe someone else has an
> > opinion about this?
> 
> We are not limited in characters in the commit log. So every useful information
> (like error output) is more than welcome.
> The only thing which needs to be shorter than a twitter post, is the title.
> Because short and clear titles help to scroll commits.
> 
> That said, don't put things which have no interest at all. Here I'd put only this:
> 	ld: test: hidden symbol `mknod' in /usr/lib64/libc_nonshared.a(mknod.oS) is referenced by DSO
>
Thanks for the tips!

I'll send a v2 with changes.

Sergio

> -- 
> Thomas

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-dev] [PATCH v2] mk: link combined shared lib using CC
  2014-10-23 15:35 [dpdk-dev] [PATCH] mk: link combined lib using CC Sergio Gonzalez Monroy
  2014-10-28 11:45 ` De Lara Guarch, Pablo
@ 2014-10-28 15:48 ` Sergio Gonzalez Monroy
  2014-10-28 16:09   ` De Lara Guarch, Pablo
  2014-12-16 18:48   ` Thomas Monjalon
  1 sibling, 2 replies; 17+ messages in thread
From: Sergio Gonzalez Monroy @ 2014-10-28 15:48 UTC (permalink / raw)
  To: dev

If we set EXTRA_CFLAGS=-O0, build fails with following error:

/usr/bin/ld: test: hidden symbol `mknod' in /usr/lib64/libc_nonshared.a(mknod.oS) is referenced by DSO

Fix: link combined shared lib using CC if LINK_USING_CC is enabled.

Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
---
 mk/rte.lib.mk      |  1 -
 mk/rte.sharelib.mk | 12 +++++++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
index d83e808..a6abd6d 100644
--- a/mk/rte.lib.mk
+++ b/mk/rte.lib.mk
@@ -63,7 +63,6 @@ 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)
diff --git a/mk/rte.sharelib.mk b/mk/rte.sharelib.mk
index c0a811a..1fac0ad 100644
--- a/mk/rte.sharelib.mk
+++ b/mk/rte.sharelib.mk
@@ -29,6 +29,8 @@
 #   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 #   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+include $(RTE_SDK)/mk/internal/rte.build-pre.mk
+
 # VPATH contains at least SRCDIR
 VPATH += $(SRCDIR)
 
@@ -45,7 +47,15 @@ sharelib: $(LIB_ONE) FORCE
 
 OBJS = $(wildcard $(RTE_OUTPUT)/build/lib/*.o)
 
-O_TO_S = $(LD) $(CPU_LDFLAGS) -shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE)
+ifeq ($(LINK_USING_CC),1)
+# Override the definition of LD here, since we're linking with CC
+LD := $(CC) $(CPU_CFLAGS)
+LD_MULDEFS := $(call linkerprefix,-z$(comma)muldefs)
+CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS))
+endif
+
+O_TO_S = $(LD) $(CPU_LDFLAGS) $(LD_MULDEFS) -shared $(OBJS)\
+	-o $(RTE_OUTPUT)/lib/$(LIB_ONE)
 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_CMD = "cmd_$@ = $(O_TO_S_STR)"
-- 
1.9.3

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH v2] mk: link combined shared lib using CC
  2014-10-28 15:48 ` [dpdk-dev] [PATCH v2] mk: link combined shared " Sergio Gonzalez Monroy
@ 2014-10-28 16:09   ` De Lara Guarch, Pablo
  2014-12-16 18:48   ` Thomas Monjalon
  1 sibling, 0 replies; 17+ messages in thread
From: De Lara Guarch, Pablo @ 2014-10-28 16:09 UTC (permalink / raw)
  To: Gonzalez Monroy, Sergio, dev



> -----Original Message-----
> From: Gonzalez Monroy, Sergio
> Sent: Tuesday, October 28, 2014 3:49 PM
> To: dev@dpdk.org
> Cc: thomas.monjalon@6wind.com; De Lara Guarch, Pablo
> Subject: [PATCH v2] mk: link combined shared lib using CC
> 
> If we set EXTRA_CFLAGS=-O0, build fails with following error:
> 
> /usr/bin/ld: test: hidden symbol `mknod' in
> /usr/lib64/libc_nonshared.a(mknod.oS) is referenced by DSO
> 
> Fix: link combined shared lib using CC if LINK_USING_CC is enabled.
> 
> Signed-off-by: Sergio Gonzalez Monroy
> <sergio.gonzalez.monroy@intel.com>

Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH v2] mk: link combined shared lib using CC
  2014-10-28 15:48 ` [dpdk-dev] [PATCH v2] mk: link combined shared " Sergio Gonzalez Monroy
  2014-10-28 16:09   ` De Lara Guarch, Pablo
@ 2014-12-16 18:48   ` Thomas Monjalon
  2014-12-16 23:42     ` Thomas Monjalon
  1 sibling, 1 reply; 17+ messages in thread
From: Thomas Monjalon @ 2014-12-16 18:48 UTC (permalink / raw)
  To: Sergio Gonzalez Monroy; +Cc: dev

2014-10-28 15:48, Sergio Gonzalez Monroy:
> If we set EXTRA_CFLAGS=-O0, build fails with following error:
> 
> /usr/bin/ld: test: hidden symbol `mknod' in /usr/lib64/libc_nonshared.a(mknod.oS) is referenced by DSO
> 
> Fix: link combined shared lib using CC if LINK_USING_CC is enabled.
> 
> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> ---
>  mk/rte.lib.mk      |  1 -
>  mk/rte.sharelib.mk | 12 +++++++++++-
[...]
> --- a/mk/rte.lib.mk
> +++ b/mk/rte.lib.mk
> @@ -63,7 +63,6 @@ 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

Why are you removing this line?

> --- a/mk/rte.sharelib.mk
> +++ b/mk/rte.sharelib.mk
[...]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH v2] mk: link combined shared lib using CC
  2014-12-16 18:48   ` Thomas Monjalon
@ 2014-12-16 23:42     ` Thomas Monjalon
  2014-12-17 10:41       ` Gonzalez Monroy, Sergio
  0 siblings, 1 reply; 17+ messages in thread
From: Thomas Monjalon @ 2014-12-16 23:42 UTC (permalink / raw)
  To: Sergio Gonzalez Monroy; +Cc: dev

2014-12-16 19:48, Thomas Monjalon:
> 2014-10-28 15:48, Sergio Gonzalez Monroy:
> > If we set EXTRA_CFLAGS=-O0, build fails with following error:
> > 
> > /usr/bin/ld: test: hidden symbol `mknod' in /usr/lib64/libc_nonshared.a(mknod.oS) is referenced by DSO
> > 
> > Fix: link combined shared lib using CC if LINK_USING_CC is enabled.
> > 
> > Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> > ---
> >  mk/rte.lib.mk      |  1 -
> >  mk/rte.sharelib.mk | 12 +++++++++++-
> [...]
> > --- a/mk/rte.lib.mk
> > +++ b/mk/rte.lib.mk
> > @@ -63,7 +63,6 @@ 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
> 
> Why are you removing this line?

If it's really needed, it could be another patch.
 
> > --- a/mk/rte.sharelib.mk
> > +++ b/mk/rte.sharelib.mk
> [...]

Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Applied without the change to rte.lib.mk.

Thanks
-- 
Thomas

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH v2] mk: link combined shared lib using CC
  2014-12-16 23:42     ` Thomas Monjalon
@ 2014-12-17 10:41       ` Gonzalez Monroy, Sergio
  2014-12-17 14:01         ` Thomas Monjalon
  0 siblings, 1 reply; 17+ messages in thread
From: Gonzalez Monroy, Sergio @ 2014-12-17 10:41 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Tuesday, December 16, 2014 11:43 PM
> 
> 2014-12-16 19:48, Thomas Monjalon:
> > 2014-10-28 15:48, Sergio Gonzalez Monroy:
> > > If we set EXTRA_CFLAGS=-O0, build fails with following error:
> > >
> > > /usr/bin/ld: test: hidden symbol `mknod' in
> > > /usr/lib64/libc_nonshared.a(mknod.oS) is referenced by DSO
> > >
> > > Fix: link combined shared lib using CC if LINK_USING_CC is enabled.
> > >
> > > Signed-off-by: Sergio Gonzalez Monroy
> > > <sergio.gonzalez.monroy@intel.com>
> > > ---
> > >  mk/rte.lib.mk      |  1 -
> > >  mk/rte.sharelib.mk | 12 +++++++++++-
> > [...]
> > > --- a/mk/rte.lib.mk
> > > +++ b/mk/rte.lib.mk
> > > @@ -63,7 +63,6 @@ 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
> >
> > Why are you removing this line?
> 
Hi Thomas,

Basically the problem is when CPU_LDFLAGS has a value, which in the currently only happens if we set CPU_LDFLAGS in the command line or build i686 (setting CPU_LDFLAGS=-melf_i386).
In those cases, given the way the makefiles are being included, CPU_LDFLAGS is being 'linkerprefixed' twice, resulting in build error.
Removing that line avoided the second linker prefix of the CPU_LDFLAGS (note that we reset the original var instead of using a different var name).

There probably is a better way but I could not see an straightforward fix for it.
I planned to improve the current build system (already sent an RFC) and was waiting until 1.8 release to resume the work on it.

Thanks,
Sergio

> If it's really needed, it could be another patch.
> 
> > > --- a/mk/rte.sharelib.mk
> > > +++ b/mk/rte.sharelib.mk
> > [...]
> 
> Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> 
> Applied without the change to rte.lib.mk.
> 
> Thanks
> --
> Thomas

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH v2] mk: link combined shared lib using CC
  2014-12-17 10:41       ` Gonzalez Monroy, Sergio
@ 2014-12-17 14:01         ` Thomas Monjalon
  2014-12-17 18:49           ` Thomas Monjalon
  0 siblings, 1 reply; 17+ messages in thread
From: Thomas Monjalon @ 2014-12-17 14:01 UTC (permalink / raw)
  To: Gonzalez Monroy, Sergio; +Cc: dev

2014-12-17 10:41, Gonzalez Monroy, Sergio:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2014-12-16 19:48, Thomas Monjalon:
> > > 2014-10-28 15:48, Sergio Gonzalez Monroy:
> > > > Fix: link combined shared lib using CC if LINK_USING_CC is enabled.
> > > >
> > > > Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> > > > ---
> > > >  mk/rte.lib.mk      |  1 -
> > > >  mk/rte.sharelib.mk | 12 +++++++++++-
> > > [...]
> > > > --- a/mk/rte.lib.mk
> > > > +++ b/mk/rte.lib.mk
> > > > @@ -63,7 +63,6 @@ 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
> > >
> > > Why are you removing this line?
> > 
> Hi Thomas,
> 
> Basically the problem is when CPU_LDFLAGS has a value, which in the
> currently only happens if we set CPU_LDFLAGS in the command line or
> build i686 (setting CPU_LDFLAGS=-melf_i386).
> In those cases, given the way the makefiles are being included,

These 2 files don't include each other.
rte.sharelib.mk is included only in lib/Makefile
and rte.lib.mk is included in each lib Makefile.

I think the problem is not about how the files are included
but how this variable is exported.
CPU_LDFLAGS is modified in lib/Makefile via rte.sharelib.mk,
then exported (see mk/arch/*/rte.vars.mk) then modified
again in libs via rte.lib.mk.

> CPU_LDFLAGS is being 'linkerprefixed' twice, resulting in build error.
> Removing that line avoided the second linker prefix of the CPU_LDFLAGS
> (note that we reset the original var instead of using a different var name).
> 
> There probably is a better way but I could not see an straightforward fix for it.

I suggest 2 possible fixes:
- do not export this variable (why is it exported?)
- or do not override the variable, i.e. use a different variable for prefixing

> I planned to improve the current build system (already sent an RFC)
> and was waiting until 1.8 release to resume the work on it.

I agree we should work on improvements after 1.8.

> > If it's really needed, it could be another patch.
[...]
> > 
> > Applied without the change to rte.lib.mk.

It definitely deserves another patch with a proper fix.

-- 
Thomas

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH v2] mk: link combined shared lib using CC
  2014-12-17 14:01         ` Thomas Monjalon
@ 2014-12-17 18:49           ` Thomas Monjalon
  2014-12-17 21:59             ` [dpdk-dev] [PATCH 0/3] mk: fix link options Thomas Monjalon
  0 siblings, 1 reply; 17+ messages in thread
From: Thomas Monjalon @ 2014-12-17 18:49 UTC (permalink / raw)
  To: Gonzalez Monroy, Sergio; +Cc: dev

2014-12-17 15:01, Thomas Monjalon:
> 2014-12-17 10:41, Gonzalez Monroy, Sergio:
> > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2014-12-16 19:48, Thomas Monjalon:
> > > > 2014-10-28 15:48, Sergio Gonzalez Monroy:
> > > > > Fix: link combined shared lib using CC if LINK_USING_CC is enabled.
> > > > >
> > > > > Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> > > > > ---
> > > > >  mk/rte.lib.mk      |  1 -
> > > > >  mk/rte.sharelib.mk | 12 +++++++++++-
> > > > [...]
> > > > > --- a/mk/rte.lib.mk
> > > > > +++ b/mk/rte.lib.mk
> > > > > @@ -63,7 +63,6 @@ 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
> > > >
> > > > Why are you removing this line?
> > > 
> > Hi Thomas,
> > 
> > Basically the problem is when CPU_LDFLAGS has a value, which in the
> > currently only happens if we set CPU_LDFLAGS in the command line or
> > build i686 (setting CPU_LDFLAGS=-melf_i386).
> > In those cases, given the way the makefiles are being included,
> 
> These 2 files don't include each other.
> rte.sharelib.mk is included only in lib/Makefile
> and rte.lib.mk is included in each lib Makefile.
> 
> I think the problem is not about how the files are included
> but how this variable is exported.
> CPU_LDFLAGS is modified in lib/Makefile via rte.sharelib.mk,
> then exported (see mk/arch/*/rte.vars.mk) then modified
> again in libs via rte.lib.mk.
> 
> > CPU_LDFLAGS is being 'linkerprefixed' twice, resulting in build error.
> > Removing that line avoided the second linker prefix of the CPU_LDFLAGS
> > (note that we reset the original var instead of using a different var name).
> > 
> > There probably is a better way but I could not see an straightforward fix for it.
> 
> I suggest 2 possible fixes:
> - do not export this variable (why is it exported?)
> - or do not override the variable, i.e. use a different variable for prefixing

I'm preparing a patch with the second approach.
It raised other problems like error when linking examples with combined library.
Patchset to come.

> > I planned to improve the current build system (already sent an RFC)
> > and was waiting until 1.8 release to resume the work on it.
> 
> I agree we should work on improvements after 1.8.
> 
> > > If it's really needed, it could be another patch.
> [...]
> > > 
> > > Applied without the change to rte.lib.mk.
> 
> It definitely deserves another patch with a proper fix.

-- 
Thomas

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-dev] [PATCH 0/3] mk: fix link options
  2014-12-17 18:49           ` Thomas Monjalon
@ 2014-12-17 21:59             ` Thomas Monjalon
  2014-12-17 21:59               ` [dpdk-dev] [PATCH 1/3] mk: fix link examples to combined library Thomas Monjalon
                                 ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Thomas Monjalon @ 2014-12-17 21:59 UTC (permalink / raw)
  To: dev

When trying to fix the use of combined library, some bugs have been
raised. This patchset fix and clean some link options.

Thomas Monjalon (3):
  mk: fix link examples to combined library
  mk: forbid multiple definitions
  mk: fix link with CC

 mk/rte.app.mk      | 5 ++---
 mk/rte.lib.mk      | 9 +++++----
 mk/rte.shared.mk   | 6 ++----
 mk/rte.sharelib.mk | 9 +++++----
 mk/rte.vars.mk     | 9 +++++----
 5 files changed, 19 insertions(+), 19 deletions(-)

-- 
2.1.3

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-dev] [PATCH 1/3] mk: fix link examples to combined library
  2014-12-17 21:59             ` [dpdk-dev] [PATCH 0/3] mk: fix link options Thomas Monjalon
@ 2014-12-17 21:59               ` Thomas Monjalon
  2014-12-17 21:59               ` [dpdk-dev] [PATCH 2/3] mk: forbid multiple definitions Thomas Monjalon
                                 ` (2 subsequent siblings)
  3 siblings, 0 replies; 17+ messages in thread
From: Thomas Monjalon @ 2014-12-17 21:59 UTC (permalink / raw)
  To: dev

RTE_LIBNAME was defined only if BUILDING_RTE_SDK.
So external applications like examples were trying to link with -l
without any library name.

This bug appeared after fixing link to combined library (removing
link to separate libraries).

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

diff --git a/mk/rte.vars.mk b/mk/rte.vars.mk
index 1e874ee..d5b36be 100644
--- a/mk/rte.vars.mk
+++ b/mk/rte.vars.mk
@@ -71,10 +71,11 @@ ifneq ($(BUILDING_RTE_SDK),)
   ifeq ($(RTE_BUILD_COMBINE_LIBS),)
     RTE_BUILD_COMBINE_LIBS := n
   endif
-  RTE_LIBNAME := $(CONFIG_RTE_LIBNAME:"%"=%)
-  ifeq ($(RTE_LIBNAME),)
-    RTE_LIBNAME := intel_dpdk
-  endif
+endif
+
+RTE_LIBNAME := $(CONFIG_RTE_LIBNAME:"%"=%)
+ifeq ($(RTE_LIBNAME),)
+RTE_LIBNAME := intel_dpdk
 endif
 
 # RTE_TARGET is deducted from config when we are building the SDK.
-- 
2.1.3

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-dev] [PATCH 2/3] mk: forbid multiple definitions
  2014-12-17 21:59             ` [dpdk-dev] [PATCH 0/3] mk: fix link options Thomas Monjalon
  2014-12-17 21:59               ` [dpdk-dev] [PATCH 1/3] mk: fix link examples to combined library Thomas Monjalon
@ 2014-12-17 21:59               ` Thomas Monjalon
  2014-12-17 21:59               ` [dpdk-dev] [PATCH 3/3] mk: fix link with CC Thomas Monjalon
  2014-12-17 23:35               ` [dpdk-dev] [PATCH 0/3] mk: fix link options Thomas Monjalon
  3 siblings, 0 replies; 17+ messages in thread
From: Thomas Monjalon @ 2014-12-17 21:59 UTC (permalink / raw)
  To: dev

The option "-z muldefs" was set only if not using ld directly.
By the way, this option seems to be a useless hack introduced
with shared and combined libraries support (e25e4d7ef16b8aa84de).
The clean approach is to remove it.

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

diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
index d83e808..a67c129 100644
--- a/mk/rte.lib.mk
+++ b/mk/rte.lib.mk
@@ -62,7 +62,6 @@ 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) $(CPU_CFLAGS)
-LD_MULDEFS := $(call linkerprefix,-z$(comma)muldefs)
 CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS))
 endif
 
@@ -75,7 +74,7 @@ O_TO_A_DO = @set -e; \
 	$(O_TO_A) && \
 	echo $(O_TO_A_CMD) > $(call exe2cmd,$(@))
 
-O_TO_S = $(LD) $(CPU_LDFLAGS) $(LD_MULDEFS) -shared $(OBJS-y) -o $(LIB)
+O_TO_S = $(LD) $(CPU_LDFLAGS) -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; \
@@ -91,7 +90,7 @@ O_TO_C_DO = @set -e; \
 	$(lib_dir) \
 	$(copy_obj)
 else
-O_TO_C = $(LD) $(LD_MULDEFS) -shared $(OBJS-y) -o $(LIB_ONE)
+O_TO_C = $(LD) -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; \
diff --git a/mk/rte.sharelib.mk b/mk/rte.sharelib.mk
index d0ec12a..d0cc7e3 100644
--- a/mk/rte.sharelib.mk
+++ b/mk/rte.sharelib.mk
@@ -50,11 +50,10 @@ OBJS = $(wildcard $(RTE_OUTPUT)/build/lib/*.o)
 ifeq ($(LINK_USING_CC),1)
 # Override the definition of LD here, since we're linking with CC
 LD := $(CC) $(CPU_CFLAGS)
-LD_MULDEFS := $(call linkerprefix,-z$(comma)muldefs)
 CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS))
 endif
 
-O_TO_S = $(LD) $(CPU_LDFLAGS) $(LD_MULDEFS) -shared $(OBJS) \
+O_TO_S = $(LD) $(CPU_LDFLAGS) -shared $(OBJS) \
 	-o $(RTE_OUTPUT)/lib/$(LIB_ONE)
 O_TO_S_STR = $(subst ','\'',$(O_TO_S)) #'# fix syntax highlight
 O_TO_S_DISP = $(if $(V),"$(O_TO_S_STR)","  LD $(@)")
-- 
2.1.3

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-dev] [PATCH 3/3] mk: fix link with CC
  2014-12-17 21:59             ` [dpdk-dev] [PATCH 0/3] mk: fix link options Thomas Monjalon
  2014-12-17 21:59               ` [dpdk-dev] [PATCH 1/3] mk: fix link examples to combined library Thomas Monjalon
  2014-12-17 21:59               ` [dpdk-dev] [PATCH 2/3] mk: forbid multiple definitions Thomas Monjalon
@ 2014-12-17 21:59               ` Thomas Monjalon
  2014-12-17 23:35               ` [dpdk-dev] [PATCH 0/3] mk: fix link options Thomas Monjalon
  3 siblings, 0 replies; 17+ messages in thread
From: Thomas Monjalon @ 2014-12-17 21:59 UTC (permalink / raw)
  To: dev

It appeared in commit 21cdc2e77a4ca999 ("fix 32-bit link with gcc")
that linker options must be prefixed by -Wl, when using CC.
So CPU_LDFLAGS is prefixed in rte.lib.mk.
Then commit 815cfb7925bb6de ("fix link of combined shared library using CC")
introduced another prefixing of CPU_LDFLAGS in rte.sharelib.mk,
included in lib/Makefile.
Because CPU_LDFLAGS is an exported variable, the prefixing is done twice.
Initial patch of commit 815cfb7925bb6de had a workaround but it hasn't
been applied in favor of this proper fix.

Now variables are not overriden when prefixing.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 mk/rte.app.mk      | 5 ++---
 mk/rte.lib.mk      | 6 ++++--
 mk/rte.shared.mk   | 6 ++----
 mk/rte.sharelib.mk | 8 +++++---
 4 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 3c35985..e1a0dbf 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -256,11 +256,10 @@ LDLIBS += -l$(RTE_LIBNAME)
 endif
 
 ifeq ($(LINK_USING_CC),1)
-LDLIBS := $(call linkerprefix,$(LDLIBS))
-LDFLAGS := $(call linkerprefix,$(LDFLAGS))
 override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS))
 O_TO_EXE = $(CC) $(CFLAGS) $(LDFLAGS_$(@)) \
-	-Wl,-Map=$(@).map,--cref -o $@ $(OBJS-y) $(LDFLAGS) $(EXTRA_LDFLAGS) $(LDLIBS)
+	-Wl,-Map=$(@).map,--cref -o $@ $(OBJS-y) $(call linkerprefix,$(LDFLAGS)) \
+	$(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS))
 else
 O_TO_EXE = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
 	-Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS)
diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
index a67c129..81bf8e1 100644
--- a/mk/rte.lib.mk
+++ b/mk/rte.lib.mk
@@ -62,7 +62,9 @@ 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) $(CPU_CFLAGS)
-CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS))
+_CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS))
+else
+_CPU_LDFLAGS := $(CPU_LDFLAGS)
 endif
 
 O_TO_A = $(AR) crus $(LIB) $(OBJS-y)
@@ -74,7 +76,7 @@ O_TO_A_DO = @set -e; \
 	$(O_TO_A) && \
 	echo $(O_TO_A_CMD) > $(call exe2cmd,$(@))
 
-O_TO_S = $(LD) $(CPU_LDFLAGS) -shared $(OBJS-y) -o $(LIB)
+O_TO_S = $(LD) $(_CPU_LDFLAGS) -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; \
diff --git a/mk/rte.shared.mk b/mk/rte.shared.mk
index 42feee7..fc6b0b4 100644
--- a/mk/rte.shared.mk
+++ b/mk/rte.shared.mk
@@ -58,11 +58,9 @@ build: _postbuild
 exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1))))
 
 ifeq ($(LINK_USING_CC),1)
-LDLIBS := $(call linkerprefix,$(LDLIBS))
-LDFLAGS := $(call linkerprefix,$(LDFLAGS))
 override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS))
-O_TO_SO = $(CC) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
-	-shared -o $@ $(OBJS-y) $(LDLIBS)
+O_TO_SO = $(CC) $(call linkerprefix,$(LDFLAGS)) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
+	-shared -o $@ $(OBJS-y) $(call linkerprefix,$(LDLIBS))
 else
 O_TO_SO = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
 	-shared -o $@ $(OBJS-y) $(LDLIBS)
diff --git a/mk/rte.sharelib.mk b/mk/rte.sharelib.mk
index d0cc7e3..de53558 100644
--- a/mk/rte.sharelib.mk
+++ b/mk/rte.sharelib.mk
@@ -50,11 +50,13 @@ OBJS = $(wildcard $(RTE_OUTPUT)/build/lib/*.o)
 ifeq ($(LINK_USING_CC),1)
 # Override the definition of LD here, since we're linking with CC
 LD := $(CC) $(CPU_CFLAGS)
-CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS))
+O_TO_S = $(LD) $(call linkerprefix,$(CPU_LDFLAGS)) \
+	-shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE)
+else
+O_TO_S = $(LD) $(CPU_LDFLAGS) \
+	-shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE)
 endif
 
-O_TO_S = $(LD) $(CPU_LDFLAGS) -shared $(OBJS) \
-	-o $(RTE_OUTPUT)/lib/$(LIB_ONE)
 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_CMD = "cmd_$@ = $(O_TO_S_STR)"
-- 
2.1.3

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH 0/3] mk: fix link options
  2014-12-17 21:59             ` [dpdk-dev] [PATCH 0/3] mk: fix link options Thomas Monjalon
                                 ` (2 preceding siblings ...)
  2014-12-17 21:59               ` [dpdk-dev] [PATCH 3/3] mk: fix link with CC Thomas Monjalon
@ 2014-12-17 23:35               ` Thomas Monjalon
  3 siblings, 0 replies; 17+ messages in thread
From: Thomas Monjalon @ 2014-12-17 23:35 UTC (permalink / raw)
  To: dev

2014-12-17 22:59, Thomas Monjalon:
> When trying to fix the use of combined library, some bugs have been
> raised. This patchset fix and clean some link options.
> 
> Thomas Monjalon (3):
>   mk: fix link examples to combined library
>   mk: forbid multiple definitions
>   mk: fix link with CC

Applied quickly to validate building before the release.

-- 
Thomas

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2014-12-17 23:36 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-23 15:35 [dpdk-dev] [PATCH] mk: link combined lib using CC Sergio Gonzalez Monroy
2014-10-28 11:45 ` De Lara Guarch, Pablo
2014-10-28 14:51   ` Sergio Gonzalez Monroy
2014-10-28 15:33     ` Thomas Monjalon
2014-10-28 15:39       ` Sergio Gonzalez Monroy
2014-10-28 15:48 ` [dpdk-dev] [PATCH v2] mk: link combined shared " Sergio Gonzalez Monroy
2014-10-28 16:09   ` De Lara Guarch, Pablo
2014-12-16 18:48   ` Thomas Monjalon
2014-12-16 23:42     ` Thomas Monjalon
2014-12-17 10:41       ` Gonzalez Monroy, Sergio
2014-12-17 14:01         ` Thomas Monjalon
2014-12-17 18:49           ` Thomas Monjalon
2014-12-17 21:59             ` [dpdk-dev] [PATCH 0/3] mk: fix link options Thomas Monjalon
2014-12-17 21:59               ` [dpdk-dev] [PATCH 1/3] mk: fix link examples to combined library Thomas Monjalon
2014-12-17 21:59               ` [dpdk-dev] [PATCH 2/3] mk: forbid multiple definitions Thomas Monjalon
2014-12-17 21:59               ` [dpdk-dev] [PATCH 3/3] mk: fix link with CC Thomas Monjalon
2014-12-17 23:35               ` [dpdk-dev] [PATCH 0/3] mk: fix link options Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).