Re: [PATCH] Revert commit 05a910f7

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

On Mon, Mar 7, 2016 at 4:13 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> 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.

Another thing, I don't think inline with _HAVE_STRING_ARCH_mempcpy
checking should be in <string.h>.  It belongs to a different header file.

-- 
H.J.