[dpdk-dev] [PATCH 1/3] eal/x86: fix build with clang for old AVX
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
[dpdk-dev] [PATCH 1/3] eal/x86: fix build with clang for old AVX
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.
[dpdk-dev] [PATCH 1/3] eal/x86: fix build with clang for old AVX
> 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
> ---
> 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
[dpdk-dev] [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
---
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
