[PATCH 0/3] fix build for default machine

[PATCH 0/3] fix build for default machine

[Thomas Monjalon]

When checking build with clang and RTE_MACHINE=default,
3 errors appeared.

Thomas Monjalon (3):
  eal/x86: fix build with clang for old AVX
  examples/l3fwd: fix build without SSE4.1
  examples/ip_pipeline: fix build for x86_64 without SSE4.2

 examples/ip_pipeline/pipeline/hash_func.h           | 2 +-
 examples/l3fwd/main.c                               | 3 ++-
 lib/librte_eal/common/include/arch/x86/rte_memcpy.h | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

-- 
2.7.0

[PATCH 1/3] eal/x86: fix build with clang for old AVX

[Thomas Monjalon]

When configuring RTE_MACHINE to "default", rte_memcpy implementation
is the default one (old AVX).
In this code, clang raises a warning thanks to -Wsometimes-uninitialized:

rte_memcpy.h:838:6: error:
variable 'srcofs' is used uninitialized whenever 'if' condition is false
        if (dstofss > 0) {
            ^~~~~~~~~~~
rte_memcpy.h:849:6: note: uninitialized use occurs here
        if (srcofs == 0) {
            ^~~~~~

It is fixed by initializing srcofs to 0.

Fixes: 1ae817f9f887 ("eal/x86: tune memcpy for platforms without AVX512")

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 lib/librte_eal/common/include/arch/x86/rte_memcpy.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
index 8e2c53c..5badfbc 100644
--- a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
+++ b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
@@ -739,7 +739,7 @@ rte_memcpy(void *dst, const void *src, size_t n)
 	uintptr_t srcu = (uintptr_t)src;
 	void *ret = dst;
 	size_t dstofss;
-	size_t srcofs;
+	size_t srcofs = 0;
 
 	/**
 	 * Copy less than 16 bytes
-- 
2.7.0

Re: [PATCH 1/3] eal/x86: fix build with clang for old AVX

[Wang, Zhihong]

> Subject: [PATCH 1/3] eal/x86: fix build with clang for old AVX
> 
> When configuring RTE_MACHINE to "default", rte_memcpy implementation
> is the default one (old AVX).
> In this code, clang raises a warning thanks to -Wsometimes-uninitialized:
> 
> rte_memcpy.h:838:6: error:
> variable 'srcofs' is used uninitialized whenever 'if' condition is false
>         if (dstofss > 0) {
>             ^~~~~~~~~~~
> rte_memcpy.h:849:6: note: uninitialized use occurs here
>         if (srcofs == 0) {
>             ^~~~~~
> 
> It is fixed by initializing srcofs to 0.
> 
> Fixes: 1ae817f9f887 ("eal/x86: tune memcpy for platforms without AVX512")
> 
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> ---
>  lib/librte_eal/common/include/arch/x86/rte_memcpy.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)


Hi Thomas,

Thanks for pointing this out!
My last hasty modification on this is not correct.

The patch below will fix it. All modifications are tested.
Sorry for all the hassle! :'(

"srcofs" should be calculated based on source address anyway.


--- a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
+++ b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
@@ -512,8 +512,9 @@ COPY_BLOCK_64_BACK31:
 	/**
 	 * Make store aligned when copy size exceeds 512 bytes
 	 */
-	dstofss = 32 - ((uintptr_t)dst & 0x1F);
+	dstofss = (uintptr_t)dst & 0x1F;
 	if (dstofss > 0) {
+		dstofss = 32 - dstofss;
 		n -= dstofss;
 		rte_mov32((uint8_t *)dst, (const uint8_t *)src);
 		src = (const uint8_t *)src + dstofss;
@@ -834,14 +835,15 @@ COPY_BLOCK_64_BACK15:
 	 * unaligned copy functions require up to 15 bytes
 	 * backwards access.
 	 */
-	dstofss = 16 - ((uintptr_t)dst & 0x0F) + 16;
+	dstofss = (uintptr_t)dst & 0x0F;
 	if (dstofss > 0) {
+		dstofss = 16 - dstofss + 16;
 		n -= dstofss;
 		rte_mov32((uint8_t *)dst, (const uint8_t *)src);
 		src = (const uint8_t *)src + dstofss;
 		dst = (uint8_t *)dst + dstofss;
-		srcofs = ((uintptr_t)src & 0x0F);
 	}
+	srcofs = ((uintptr_t)src & 0x0F);
 
 	/**
 	 * For aligned copy

Re: [PATCH 1/3] eal/x86: fix build with clang for old AVX

[Thomas Monjalon]

2016-02-04 03:35, Wang, Zhihong:
> > Subject: [PATCH 1/3] eal/x86: fix build with clang for old AVX
> > 
> > When configuring RTE_MACHINE to "default", rte_memcpy implementation
> > is the default one (old AVX).
> > In this code, clang raises a warning thanks to -Wsometimes-uninitialized:
> > 
> > rte_memcpy.h:838:6: error:
> > variable 'srcofs' is used uninitialized whenever 'if' condition is false
> >         if (dstofss > 0) {
> >             ^~~~~~~~~~~
> > rte_memcpy.h:849:6: note: uninitialized use occurs here
> >         if (srcofs == 0) {
> >             ^~~~~~
> > 
> > It is fixed by initializing srcofs to 0.
> > 
> > Fixes: 1ae817f9f887 ("eal/x86: tune memcpy for platforms without AVX512")
> 
> Hi Thomas,
> 
> Thanks for pointing this out!
> My last hasty modification on this is not correct.
> 
> The patch below will fix it. All modifications are tested.
> Sorry for all the hassle! :'(
> 
> "srcofs" should be calculated based on source address anyway.

OK
Please send a full patch as usual, thanks.

Re: [PATCH 1/3] eal/x86: fix build with clang for old AVX

[Thomas Monjalon]

2016-02-04 09:47, Thomas Monjalon:
> 2016-02-04 03:35, Wang, Zhihong:
> > > Subject: [PATCH 1/3] eal/x86: fix build with clang for old AVX
> > > 
> > > When configuring RTE_MACHINE to "default", rte_memcpy implementation
> > > is the default one (old AVX).
> > > In this code, clang raises a warning thanks to -Wsometimes-uninitialized:
> > > 
> > > rte_memcpy.h:838:6: error:
> > > variable 'srcofs' is used uninitialized whenever 'if' condition is false
> > >         if (dstofss > 0) {
> > >             ^~~~~~~~~~~
> > > rte_memcpy.h:849:6: note: uninitialized use occurs here
> > >         if (srcofs == 0) {
> > >             ^~~~~~
> > > 
> > > It is fixed by initializing srcofs to 0.
> > > 
> > > Fixes: 1ae817f9f887 ("eal/x86: tune memcpy for platforms without AVX512")
> > 
> > Hi Thomas,
> > 
> > Thanks for pointing this out!
> > My last hasty modification on this is not correct.
> > 
> > The patch below will fix it. All modifications are tested.
> > Sorry for all the hassle! :'(
> > 
> > "srcofs" should be calculated based on source address anyway.
> 
> OK
> Please send a full patch as usual, thanks.

Sorry I've just caught you have already sent it. Thanks

[PATCH 2/3] examples/l3fwd: fix build without SSE4.1

[Thomas Monjalon]

clang reports this error:
examples/l3fwd/main.c:550:1: error: unused function 'send_packetsx4'

The function is used only when ENABLE_MULTI_BUFFER_OPTIMIZE is 1.

Fixes: 96ff445371e0 ("examples/l3fwd: reorganise and optimize LPM code path")
Fixes: 6f1c1e28d98e ("examples/l3fwd: fix build with exact-match enabled")

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 examples/l3fwd/main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
index 21a5782..a93497d 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -545,7 +545,8 @@ send_single_packet(struct rte_mbuf *m, uint8_t port)
 	return 0;
 }
 
-#if (APP_LOOKUP_METHOD == APP_LOOKUP_LPM)
+#if ((APP_LOOKUP_METHOD == APP_LOOKUP_LPM) && \
+	(ENABLE_MULTI_BUFFER_OPTIMIZE == 1))
 static inline __attribute__((always_inline)) void
 send_packetsx4(struct lcore_conf *qconf, uint8_t port,
 	struct rte_mbuf *m[], uint32_t num)
-- 
2.7.0

Re: [PATCH 2/3] examples/l3fwd: fix build without SSE4.1

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, February 03, 2016 6:57 PM
> To: dev@dpdk.org
> Cc: Ananyev, Konstantin; Wang, Zhihong; De Lara Guarch, Pablo; Dumitrescu,
> Cristian
> Subject: [PATCH 2/3] examples/l3fwd: fix build without SSE4.1
> 
> clang reports this error:
> examples/l3fwd/main.c:550:1: error: unused function 'send_packetsx4'
> 
> The function is used only when ENABLE_MULTI_BUFFER_OPTIMIZE is 1.
> 
> Fixes: 96ff445371e0 ("examples/l3fwd: reorganise and optimize LPM code
> path")
> Fixes: 6f1c1e28d98e ("examples/l3fwd: fix build with exact-match enabled")
> 
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>

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

[PATCH 3/3] examples/ip_pipeline: fix build for x86_64 without SSE4.2

[Thomas Monjalon]

The compiler cannot use _mm_crc32_u64:

examples/ip_pipeline/pipeline/hash_func.h:165:9:
error: implicit declaration of function '_mm_crc32_u64' is invalid in C99

Fixes: 947024a26df7 ("examples/ip_pipeline: rework passthrough pipeline")

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 examples/ip_pipeline/pipeline/hash_func.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/ip_pipeline/pipeline/hash_func.h b/examples/ip_pipeline/pipeline/hash_func.h
index 7846300..1953ad4 100644
--- a/examples/ip_pipeline/pipeline/hash_func.h
+++ b/examples/ip_pipeline/pipeline/hash_func.h
@@ -152,7 +152,7 @@ hash_xor_key64(void *key, __rte_unused uint32_t key_size, uint64_t seed)
 	return (xor0 >> 32) ^ xor0;
 }
 
-#if defined(__x86_64__)
+#if defined(__x86_64__) && defined(RTE_CPUFLAG_SSE4_2)
 
 #include <x86intrin.h>
 
-- 
2.7.0

Re: [PATCH 0/3] fix build for default machine

[Thomas Monjalon]

2016-02-03 19:56, Thomas Monjalon:
> When checking build with clang and RTE_MACHINE=default,
> 3 errors appeared.
> 
> Thomas Monjalon (3):
>   eal/x86: fix build with clang for old AVX
>   examples/l3fwd: fix build without SSE4.1
>   examples/ip_pipeline: fix build for x86_64 without SSE4.2

Patch 1 is rejected, because fixed differently:
http://dpdk.org/ml/archives/dev/2016-February/032718.html
Others are re-sent in this series:
http://dpdk.org/ml/archives/dev/2016-February/033223.html