[libc] Always use __builtin_wasm_memory_copy in memmove

[sbc100]

Followup to #27653.

Replace Musl's memmove.c and the -Oz manual loop in emscripten_memmove.c with __builtin_wasm_memory_copy.

WebAssembly's memory.copy instruction is specified with overlapping copy (memmove) semantics, and engines (V8, SpiderMonkey, JSC) implement it using native host memmove.

In Musl's C implementation, misaligned overlapping copies fall back to a byte-by-byte scalar loop in wasm, which is 15x to 33x slower than memory.copy.

Overlapping copy benchmark results (200k iterations across sizes 1B to 16KB):

Shift forward (dest > src):

• V8: 34.51ms vs 576.50ms (94.0% faster, 16.7x)
• SpiderMonkey: 33.73ms vs 1129.64ms (97.0% faster, 33.5x)
• JavaScriptCore: 33.32ms vs 715.26ms (95.3% faster, 21.5x)

Shift backward (dest < src):

• V8: 34.28ms vs 551.30ms (93.8% faster, 16.1x)
• SpiderMonkey: 33.74ms vs 546.25ms (93.8% faster, 16.2x)
• JavaScriptCore: 32.90ms vs 535.86ms (93.9% faster, 16.3x)

[kripken]

Where is the benchmark?

[sbc100]

AI-Generated: memmove Benchmark Details & Workloads

Here is the microbenchmark used to measure __builtin_wasm_memory_copy vs Musl's memmove implementation for overlapping copies:

Source Code (compare_memmove.c)

#include <stdint.h>
#include <string.h>
#include <stdio.h>
#include <emscripten/emscripten.h>

char dst[1024*1024*16+16] = {};
volatile uint8_t checksum = 0;

typedef __attribute__((__may_alias__)) size_t WT;
#define WS (sizeof(WT))

// Musl's memmove implementation
void *__attribute__((noinline)) memmove_musl(void *dest, const void *src, size_t n)
{
	char *d = dest;
	const char *s = src;

	if (d==s) return d;
	if ((uintptr_t)s-(uintptr_t)d-n <= -2*n) {
          if (n) __builtin_wasm_memory_copy(0, 0, d, s, n);
          return dest;
        }

	if (d<s) {
		if ((uintptr_t)s % WS == (uintptr_t)d % WS) {
			while ((uintptr_t)d % WS) {
				if (!n--) return dest;
				*d++ = *s++;
			}
			for (; n>=WS; n-=WS, d+=WS, s+=WS) *(WT *)d = *(WT *)s;
		}
		for (; n; n--) *d++ = *s++;
	} else {
		if ((uintptr_t)s % WS == (uintptr_t)d % WS) {
			while ((uintptr_t)(d+n) % WS) {
				if (!n--) return dest;
				d[n] = s[n];
			}
			while (n>=WS) n-=WS, *(WT *)(d+n) = *(WT *)(s+n);
		}
		while (n) n--, d[n] = s[n];
	}

	return dest;
}

// Builtin memory.copy implementation
void *__attribute__((noinline)) memmove_bulkmem(void *dest, const void *src, size_t n) {
  if (n) {
    __builtin_wasm_memory_copy(0, 0, dest, src, n);
  }
  return dest;
}

int sizes[] = {1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 4096, 16384};
int num_sizes = sizeof(sizes)/sizeof(sizes[0]);

int main() {
  const int iters = 100000;

  // Overlapping test: dest = buf + 1, src = buf (shift forward)
  double t0 = emscripten_get_now();
  for (int s = 0; s < num_sizes; s++) {
    int sz = sizes[s];
    for (int i = 0; i < iters; i++) {
      memmove_musl(dst + 1, dst, sz);
      checksum += dst[sz >> 1];
    }
  }
  double t_old_fwd = emscripten_get_now() - t0;

  t0 = emscripten_get_now();
  for (int s = 0; s < num_sizes; s++) {
    int sz = sizes[s];
    for (int i = 0; i < iters; i++) {
      memmove_bulkmem(dst + 1, dst, sz);
      checksum += dst[sz >> 1];
    }
  }
  double t_new_fwd = emscripten_get_now() - t0;

  // Overlapping test: dest = buf, src = buf + 1 (shift backward)
  t0 = emscripten_get_now();
  for (int s = 0; s < num_sizes; s++) {
    int sz = sizes[s];
    for (int i = 0; i < iters; i++) {
      memmove_musl(dst, dst + 1, sz);
      checksum += dst[sz >> 1];
    }
  }
  double t_old_bwd = emscripten_get_now() - t0;

  t0 = emscripten_get_now();
  for (int s = 0; s < num_sizes; s++) {
    int sz = sizes[s];
    for (int i = 0; i < iters; i++) {
      memmove_bulkmem(dst, dst + 1, sz);
      checksum += dst[sz >> 1];
    }
  }
  double t_new_bwd = emscripten_get_now() - t0;

  printf("Shift forward (dest > src, overlapping):\n");
  printf("  Musl memmove:    %.2f ms\n", t_old_fwd);
  printf("  bulkmem copy:    %.2f ms\n", t_new_fwd);
  printf("  Speedup:         %.1f%% %s\n", (1.0 - t_new_fwd/t_old_fwd)*100.0, t_new_fwd <= t_old_fwd ? "faster" : "slower");

  printf("Shift backward (dest < src, overlapping):\n");
  printf("  Musl memmove:    %.2f ms\n", t_old_bwd);
  printf("  bulkmem copy:    %.2f ms\n", t_new_bwd);
  printf("  Speedup:         %.1f%% %s\n", (1.0 - t_new_bwd/t_old_bwd)*100.0, t_new_bwd <= t_old_bwd ? "faster" : "slower");

  return 0;
}

Compilation & Runner

emcc -O3 compare_memmove.c -o compare_memmove.js -sINITIAL_MEMORY=64MB -sENVIRONMENT=node,shell

# Engines:
~/.jsvu/bin/v8 --no-liftoff compare_memmove.js
~/.jsvu/bin/spidermonkey compare_memmove.js
~/.jsvu/bin/javascriptcore compare_memmove.js

[kripken]

Thanks!

Interestingly I see musl is much faster when inlining is enabled, in fact, 1-2% faster than the builtin. I guess it can specialize the code for the constants in the benchmark. That does suggest to me that this is probably not much of a win for LTO, but still seems worthwhile for size.

[sbc100]

Thanks!

Interestingly I see musl is much faster when inlining is enabled, in fact, 1-2% faster than the builtin. I guess it can specialize the code for the constants in the benchmark. That does suggest to me that this is probably not much of a win for LTO, but still seems worthwhile for size.

Ah, so these number should really include LTO I guess?

But.. these files are explicitly excluded from LTO in because they are part of get_libcall_files in system_libs.py.

But..... I guess binaryen is doing inlining in -O3 anyway, so why do my number all show the builtin being faster?

[kripken]

Hmm, looks like when LLVM inlines here it ends up generating code with calls to memory.copy (edit: which is something binaryen doesn't do)

[sbc100]

I'll re-run the numbers with the memmove implementation in the separate non-LTO compilation unit (mimicking our libc setup).

[sbc100]

The benchmark above uses __attribute__((noinline)).. is that not enough to prevent llvm inlining?

[kripken]

Oh, it does prevent inlining. I experimented with removing that, to see what the inlined result would be. I was assuming that would reflect the LTO case.

But IIUC you said these files were not part of LTO? (why?)

[sbc100]

Oh, it does prevent inlining. I experimented with removing that, to see what the inlined result would be. I was assuming that would reflect the LTO case.

But IIUC you said these files were not part of LTO? (why?)

Certain symbols in compiler-rt / libc are considered "libcalls"s, which mean llvm can generate calls to them during LTO. Such functions cannot themselves be part of LTO.

See the long comment in get_libcall_files in system_libs.py.

[sbc100]

Did you find any cases there this change is not actually an improvement? even if inlining is enabled (or we make these function LTO) this change is never a regression is it?

[kripken]

Given that LTO was not optimizing these, I see no regression.

If LTO was doing so, this might have been a tiny 1-2% regression, as mentioned above.

Really a shame LTO can't handle this kind of thing. It is exactly a great candidate for an LTO speedup... I guess this PR gets us 98-99% of that benefit without LTO though.

[dschuff]

Couldn't we optimize this in principle? We don't actually need to prevent LTO from inlining this, we just need to ensure that an implementation of memcpy is available at native link time in addition to whatever LTO might do. I get that linking is already complicated though.

Another idea, I wonder if there's some way to improve the optimizer. If it can inline the other memcpy implementation and end up with better code, why isn't it doing that with this replacement? e.g., why is it seeing this implementation and not a memcpy intrinsic (which it would presumably understand better than this bare instruction)? I guess none of that matters if we're not letting LTO see the implementation in the first place.

[kleisauke]

Is this check still needed? IIUC, this zero-length check should no longer be necessary after PR llvm/llvm-project#112617.

[sbc100]

My understanding is that __builtin_wasm_memory_copy is a lower level intrinsic that lowers to just the single wasm instruction.

See the comment in the referenced PR about // Use MEMCPYhere instead ofMEMORY_COPY..

One is __builtin_memcpy (with memcpy semantics) and other is __builtin_wasm_memory_copy (single wasm instruction).

Also IIRC, one cannot use __builtin_memcpy in the implementaion of the memcpy libcall because that will lead to an infinite self-call (since __builtin_memcpy is itself allowed to lower to memcpy).

[kripken]

If it can inline the other memcpy implementation and end up with better code, why isn't it doing that with this replacement?

I was testing the benchmark here, which put the memmove impl in the same file, as just some plain C code. The optimizer can inline that in the same compilation unit, then specialize for the specific sizes we are memmoving etc.

But, in contrast, the intrinsic is "opaque" even if it is put inside this file. That is, it sees a call to an intrinsic and stops there. But, perhaps LLVM could actually see __builtin_wasm_memory_copy(x, y, 2) and emit an unrolled loop of size 2? I am pretty sure it does that for libc builtins but I guess we just don't have optimization logic for these wasm intrinsics? (binaryen might also be another place to optimize these)

[sbc100]

Couldn't we optimize this in principle? We don't actually need to prevent LTO from inlining this, we just need to ensure that an implementation of memcpy is available at native link time in addition to whatever LTO might do. I get that linking is already complicated though.

In theory I think believe the bug in question (references in the comment in get_libcall_files) maybe fixable (https://bugs.llvm.org/show_bug.cgi?id=44353). It probably just requires some attention and bunch of testing. Often the bugs that result from this only get found when real users hit them in some odd configuration, so while I would like to think our test suite covers all the cases I'm not totally convinced that passing all the tests means the issue not present.

[dschuff]

If it can inline the other memcpy implementation and end up with better code, why isn't it doing that with this replacement?

But, in contrast, the intrinsic is "opaque" even if it is put inside this file. That is, it sees a call to an intrinsic and stops there. But, perhaps LLVM could actually see __builtin_wasm_memory_copy(x, y, 2) and emit an unrolled loop of size 2? I am pretty sure it does that for libc builtins but I guess we just don't have optimization logic for these wasm intrinsics? (binaryen might also be another place to optimize these)

Yeah I was thinking that most instances of memcpy should show up in the IR as llvm.memcpy (and not calls to this code) which can then be unrolled however the optimizer likes. It seems like it should be rare in practice that it should even make a difference.

[sbc100]

Ok that land this ?

[kripken]

lgtm to land.

If we can find a way to optimize further that sounds good, but it may really be just 1-2% on memmove operations, which is likely very small in real-world programs.

[sbc100]

lgtm to land.

If we can find a way to optimize further that sounds good, but it may really be just 1-2% on memmove operations, which is likely very small in real-world programs.

Yes, in practice LLVM will already have unrolled/inlined/codegened calls to @llvm.memmove. It should only use the libcall when it chooses not to use the unrolled/inlined/codegened version. The gap I supposed is when LTO might come along and then make a slightly different inlining decision to the one maybe the the @llvm.memmove lowering code.