[PATCH] shrink last_char_is function even more

Denys Vlasenko vda.linux at googlemail.com
Sun Jul 19 18:48:10 UTC 2020


On Tue, Jul 7, 2020 at 9:17 PM Tito <farmatito at tiscali.it> wrote:
> Hi,
> attached you will find a patch that shrinks libbb's last_char_is function.
> bloatcheck is:
>
> function                                             old     new   delta
> last_char_is                                          53      42     -11
> ------------------------------------------------------------------------------
> (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-11)             Total: -11 bytes
>    text    data     bss     dec     hex filename
>  981219   16907    1872  999998   f423e busybox_old
>  981208   16907    1872  999987   f4233 busybox_unstripped
>
> Patch for review:
>
> --- libbb/last_char_is.c.orig   2020-07-05 09:54:24.737931000 +0200
> +++ libbb/last_char_is.c        2020-07-06 14:29:27.768898574 +0200
> @@ -11,14 +11,7 @@
>  /* Find out if the last character of a string matches the one given */
>  char* FAST_FUNC last_char_is(const char *s, int c)
>  {
> -       if (s) {
> -               size_t sz = strlen(s);
> -               /* Don't underrun the buffer if the string length is 0 */
> -               if (sz != 0) {
> -                       s += sz - 1;
> -                       if ((unsigned char)*s == c)
> -                               return (char*)s;
> -               }
> -       }
> -       return NULL;
> -}
> +       if (!s || !*s) return NULL;
> +    while (*(s + 1)) s++;
> +       return (*s == c) ? (char *) s : NULL;
> +}

Code size for this particular fragment depends on what stupid things
compiler feels compelled to do. My compiler does fewer of them
when I use "*s == (char)c" instead.

Also, we can drop s == NULL case, none of the callers need that to work
(similarly to all the usual string functions not expected to work on NULLs).

So...

char* FAST_FUNC last_char_is(const char *s, int c)
{
        if (!s[0])
                return NULL;
        while (s[1])
                s++;
        return (*s == (char)c) ? (char *) s : NULL;
}

   0:   89 c1                   mov    %eax,%ecx
   2:   80 38 00                cmpb   $0x0,(%eax)
   5:   74 12                   je     19 <last_char_is+0x19>
   7:   80 79 01 00             cmpb   $0x0,0x1(%ecx)
   b:   74 03                   je     10 <last_char_is+0x10>
   d:   41                      inc    %ecx
   e:   eb f7                   jmp    7 <last_char_is+0x7>
  10:   31 c0                   xor    %eax,%eax
  12:   38 11                   cmp    %dl,(%ecx)
  14:   75 05                   jne    1b <last_char_is+0x1b>
  16:   89 c8                   mov    %ecx,%eax
  18:   c3                      ret
  19:   31 c0                   xor    %eax,%eax
  1b:   c3                      ret

Almost perfect. If it'd also realize that copying EAX to ECX
and back can be avoided...


More information about the busybox mailing list