Re: [PATCH v3] wcrtomb: Make behavior POSIX compliant

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 13/05/2022 08:38, Florian Weimer via Libc-alpha wrote:
> * Siddhesh Poyarekar:
> 
>> On 13/05/2022 10:58, Paul Eggert wrote:
>>> On 5/12/22 21:56, Paul Eggert wrote:
>>>> Hope you don't mind my bikeshedding.
>>> Better yet, this:
>>>    s[0] = buf[0];
>>>    if (2 <= result && result <= 4)
>>>      {
>>>        s[1] = buf[1];
>>>        memcpy (&s[result - 2], &buf[result - 2], 2);
>>>      }
>>>    else
>>>      memcpy (s, buf, result);
>>> On x86-64 with GCC 12.1 -O2 and a glibc-supplied charmap, this is
>>> only 9 straight-line instructions, counting the compare insn and the 
>>> conditional-branch insn that is never taken.
>>>
>>
>> Sorry I missed this one.  I tried it and with gcc 11 it seems to
>> produce worse code, merging the two memcpys instead of inlining the
>> first one.
> 
> Can we please use the simplified code?
> 
> “char buf[MB_LEN_MAX];” already gives GCC a strong hint this is a short
> memcpy.  If it is beneficial on some architectures to inline such short,
> but still variable memcpy calls, we should teach the compiler how to do
> it, and not do it manually.

Totally agreed.

> 
> And wcrtomb would for sure benefit more from a fast path for ASCII and
> UTF-8. 8-p

And it does make sene.

> 
> (Not that I think there are many users out there, given glibc's
> historically poor performance in the multibyte/wide string conversion
> space.)