[PATCH] D71786: RFC: [Support] On Windows, add optional support for {rpmalloc|snmalloc|mimalloc}

Tue Aug 4 05:28:49 PDT 2020

hans added a comment.

>From my point of view, I think it would be fine to only support rpmalloc in this patch. One aspect that Russell touched on is that if we support a bunch of options, it's not clear how well that support will be tested. Supporting only rpmalloc would also simplify the patch a little, and small incremental change is always good I think.

As for the "why rpmalloc and not my favorite allocator" question - well you're doing the work, so you get to choose the allocator :-) And, if there really is strong demand for supporting a different allocator, that can be handled later.

It would be nice if rpmalloc could live as third_party code in the llvm tree eventually, so that this could just work out of the box, but until then I think the current patch is a good solution.

================
Comment at: llvm/lib/Support/CMakeLists.txt:57
+if(LLVM_INTEGRATED_CRT_ALLOC MATCHES "rpmalloc$")
+  set(ALLOCATOR_FILES "${LLVM_INTEGRATED_CRT_ALLOC}/rpmalloc/rpmalloc.c" "${LLVM_INTEGRATED_CRT_ALLOC}/rpmalloc/malloc.c")
+elseif(LLVM_INTEGRATED_CRT_ALLOC MATCHES "snmalloc$")
----------------
I thought rpmalloc/malloc.c isn't supposed to be compiled on its own, but gets included into rpmalloc.c when ENABLE_OVERRIDE is defined. I'm guessing the could explain Russell's compiler error above.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71786/new/

https://reviews.llvm.org/D71786