Re: [RFC PATCH v1 3/5] tools/nolibc: x86-64: Use `rep cmpsb` for `memcmp()`

From: Willy Tarreau
Date: Thu Aug 31 2023 - 23:37:23 EST


On Fri, Sep 01, 2023 at 10:24:42AM +0700, Ammar Faizi wrote:
> On Wed, Aug 30, 2023 at 11:26:57PM +0200, Willy Tarreau wrote:
> > Out of curiosity, given that you implemented the 3 other ones directly
> > in an asm statement, is there a particular reason this one mixes a bit
> > of C and asm ?
>
> Because this one maybe unused. The other are explicitly exported.

Makes sense, indeed.

> > It would probably be something around this, in the same vein:
> >
> > memcmp:
> > xchg %esi,%eax // source1
> > mov %rdx,%rcx // count
> > rep cmpsb // source2 in rdi; sets ZF on equal, CF if src1<src2
> > seta %al // 0 if src2 <= src1, 1 if src2 > src1
> > sbb $0, %al // 0 if src2 == src1, -1 if src2 < src1, 1 if src2 > src1
> > movsx %al, %eax // sign extend to %eax
> > ret
> >
> > Note that the output logic could have to be revisited, I'm not certain but
> > at first glance it looks valid.
>
> After thinking about this more, I think I'll drop the memcmp() patch
> because it will prevent optimization when comparing a small value.
>
> For example, without __asm__:
>
> memcmp(var, "abcd", 4);
>
> may compile to:
>
> cmpl $0x64636261, %reg
> ...something...
>
> But with __asm__, the compiler can't do that. Thus, it's not worth
> optimizing the memcmp() in this case.

Ah you're totally right!

Willy