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

[Paul Eggert <eggert@cs.ucla.edu>]

On 5/12/22 06:15, Siddhesh Poyarekar wrote:
> +	  switch (result)
> +	    {
> +	    case 4:
> +	      s[3] = buf[3];
> +	      /* Fall through.  */
> +	    case 3:
> +	      s[2] = buf[2];
> +	      /* Fall through.  */
> +	    case 2:
> +	      s[1] = buf[1];
> +	      /* Fall through.  */
> +	    case 1:
> +	      s[0] = buf[0];
> +	      break;
> +	    default:
> +	      memcpy (s, buf, result);
> +	    }

For me with GCC 12.1 -O2 on x86-64, the above code generates 2 
comparisons and 3 conditional branches in the common case where RESULT 
is 1. Plus, GCC generates a jmp from the end of case 3 to the start of 
case 2 (which precedes case 3 in the machine code), which is a bit odd.

How about something like the following instead? This generates machine 
code with only one conditional branch - the one that decides whether to 
call memcpy - and this branch is never taken with glibc-supplied 
charmaps. (I'm assuming RESULT is at least 1.)

   s[0] = buf[0];
   if (2 <= result && result <= 4)
     {
       s[1] = buf[1];
       memcpy (&s[result != 2], &buf[result != 2], 2);
       s[result - 1] = buf[result - 1];
     }
   else
     memcpy (s, buf, result);

Hope you don't mind my bikeshedding. (PATCH v3 looks correct as-is, for 
what it's worth.)