Re: [PATCH] Revert commit 05a910f7

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Mon, Mar 7, 2016 at 3:20 PM, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
> H.J. Lu <hjl.tools@gmail.com> wrote:
>>> +/* Don't inline mempcpy into memcpy as x86 has an optimized mempcpy.  */
>>> +# define _HAVE_STRING_ARCH_mempcpy 1
>>> +
>>>  /* Copy N bytes of SRC to DEST.  */
>>>  # define _HAVE_STRING_ARCH_memcpy 1
>>>  # define memcpy(dest, src, n) \
>>> --
>>> 2.5.0
>>>
>>
>> It doesn't work since  <bits/string.h> is included only if
>>
>> #if defined __GNUC__ && __GNUC__ >= 2
>> # if defined __OPTIMIZE__ && !defined __OPTIMIZE_SIZE__ \
>>      && !defined __NO_INLINE__ && !defined __cplusplus
>>
>> is true.
>>
>
>> I believe commit 05a910f7 was wrong.  At minimum,
>> mempcpy shouldn't be inlined in 2 different header files.
>
> There is nothing wrong with that commit. I already posted patches that remove most
> of the redundant stuff from bits/string.h, including the duplicate mempcpy defines.
> I don't understand how defining _HAVE_STRING_ARCH_mempcpy doesn't work for you
> either, unless you use non-standard options or a very ancient compiler.

I can make it to work.

If we don't want to wait before the other mempcpy inline is removed first,
we can put the new mempcpy inline in a new header file and x86 won't
include it until the other mempcpy inline is removed.  It is very odd
to have mempcpy inlined in 2 different places.

> The proper solution is to get rid of the bits/string.h mess altogether rather than
> conditionally including it. With my outstanding patches we're there most of the way.
>
> Wilco
>

The remove patch should have gone in first before adding another one.

-- 
H.J.