[dpdk-dev] [PATCH v3] mempool: replace c memcpy code semantics with optimized rte_memcpy

[Ferruh Yigit]

On 7/5/2016 3:09 PM, Ferruh Yigit wrote:
> On 7/5/2016 2:13 PM, Jerin Jacob wrote:
>> On Tue, Jul 05, 2016 at 07:32:46PM +0800, Yuanhan Liu wrote:
>>> On Tue, Jul 05, 2016 at 09:43:03AM +0100, Ferruh Yigit wrote:
>>>> On 6/30/2016 6:28 PM, Thomas Monjalon wrote:
>>>>> 2016-06-30 17:46, Jerin Jacob:
>>>>>> Signed-off-by: Jerin Jacob <jerin.jacob at caviumnetworks.com>
>>>>>> Acked-by: Olivier Matz <olivier.matz at 6wind.com>
>>>>>
>>>>> Applied, thanks
>>>>>
>>>>
>>>> Hi Jerin,
>>>>
>>>> This commit cause a compilation error on target i686-native-linuxapp-gcc
>>>> with gcc6.
>>>
>>> Besides that, I'm more curious to know have you actually seen any
>>> performance boost?
>>
>> let me first address your curiosity,
>> http://dpdk.org/dev/patchwork/patch/12993/( check the second comment)
>> http://dpdk.org/ml/archives/dev/2016-June/042701.html
>>
>> Ferruh,
> 
> Hi Jerin,
> 
>>
>> I have tested on a x86 machine with gcc 6.1. I could n't see any issues
>> with i686-native-linuxapp-gcc target
> Thanks for investigating the issue.
> 
>>
>> Steps following to create gcc 6.1 toolchain
>> https://sahas.ra.naman.ms/2016/05/31/building-gcc6-1-on-fedora-23/
>> (removed --disable-multilib to have support for -m32)
>>
>> ➜ [dpdk-master] $ gcc -v
>> Using built-in specs.
>> COLLECT_GCC=gcc
>> COLLECT_LTO_WRAPPER=/opt/gcc-6.1.0/libexec/gcc/x86_64-pc-linux-gnu/6.1.0/lto-wrapper
>> Target: x86_64-pc-linux-gnu
>> Configured with: ../gcc-6.1.0/configure --prefix=/opt/gcc-6.1.0
>> --enable-languages=c,c++ --enable-libmudflap --with-system-zlib
>> Thread model: posix
>> gcc version 6.1.0 (GCC)
> I am using Fedora24, which has gcc6 (6.1.1) as default.
> 
>>
>> More over this issue seems like an issue from x86 rte_memcpy implementation.
> You are right. But i686 compilation starts failing with this commit.
> And reverting this commit in the current HEAD solves the compilation
> problem.
> I am not really clear about reason of the compilation error.

The compile error is because compiler is so smart now and at the same
time not enough smart.

Call stack is as following:

virtio_xmit_pkts_simple
  virtio_xmit_cleanup
    rte_mempool_put_bulk
      rte_mempool_generic_put
        __mempool_generic_put
	  rte_memcpy

The array used as source buffer in virtio_xmit_cleanup (free) is a
pointer array with 32 elements, in 32bit this makes 128 bytes.

in rte_memcpy() implementation, there a code piece as following:
if (size > 256) {
    rte_move128(...);
    rte_move128(...); <--- [1]
    ....
}

The compiler traces the array all through the call stack and knows the
size of array is 128 and generates a warning on above [1] which tries to
access beyond byte 128.
But unfortunately it ignores the "(size > 256)" check.

In 64bit, this warning is not generated because array size becomes 256
bytes.

So this warning is a false positive. Although I am working on it, if
anybody has a suggestion to prevent warning, quite welcome. At worst I
will disable this compiler warning for the file.

Thanks,
ferruh