[PATCH] D106769: [MemCpyOpt] Relax libcall checks

Alina Sbirlea via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 26 15:16:42 PDT 2021


asbirlea added a comment.

Just my thoughts, I may not have all the info here..

I think materializing memcpy and memset should be standard and not be an issue in either sanitizers or downstream backends. There is a very small subset of intrinsics where I'd expect this, where passes introducing them are already in the default pipeline (e.g. MemCpyOpt, LoopIdiomRecognize).
I also understand the push back on the reverted patch, in that it was changing the status quo as far as expectations and a standardization should be done with proper RFC and llvm-dev discussions. I just think that this is something to consider changing in the long run.

This patch and the flag in D106401 <https://reviews.llvm.org/D106401> makes sense to resolve the issues for the NVPTX use case (and AMDGPU as well I assume).



================
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
+
----------------
tra wrote:
> 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.
Either folding into this or as a separate patch is reasonable IMO. 
@tra: do you have another test that would be relevant to add in D106401 for nvptx (beside an additional RUN line in this one with the flag I mean)? 


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

https://reviews.llvm.org/D106769



More information about the llvm-commits mailing list