[PATCH] D106769: [MemCpyOpt] Relax libcall checks

[Artem Belevich via Phabricator via llvm-commits]

tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

LGTM in general.



================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:677
+      // Don't introduce calls to memcpy/memmove intrinsics out of thin air if
+      // the corresponding libcalls are not available.
+      if (T->isAggregateType() && TLI->has(LibFunc_memcpy) &&
----------------
I'd add a TODO/FIXME. The only reason we need to do it is because of the assumption in some places (sanitizeers and some downstream back-ends) that we can't handle LLVM intrinsic w/o the matching libcall. Materializing LLVM intrinsics is not illegal and other passes do it.


================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:803
+  // The following code creates memset intrinsics out of thin air. Don't do
+  // this if the corresponding libfunc is not available.
+  if (!TLI->has(LibFunc_memset))
----------------
Ditto.


================
Comment at: llvm/test/Transforms/MemCpyOpt/no-libcalls.ll:3
+; RUN: opt -S -memcpyopt < %s | FileCheck %s --check-prefixes=CHECK,LIBCALLS
+; RUN: opt -S -memcpyopt -mtriple=amdgcn-- < %s | FileCheck %s --check-prefixes=CHECK,NO-LIBCALLS
+
----------------
This is a bit ironic as AMDGPU back-end (as well as NVPTX) *can* actually handle memory intrinsics w/o the matching libcalls.

https://godbolt.org/z/Koedbzn5x

I think it may make sense to fold D106401 into this patch and add another test for AMDGPU + all optimizations enabled by the flag.


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

https://reviews.llvm.org/D106769