[PATCH] D52081: [InstCombine] do not expand 8 byte memcpy if optimising for minsize

Sjoerd Meijer via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 17 01:15:59 PDT 2018


SjoerdMeijer added a comment.

Thanks for the feedback and suggestions! Summarising where we are:

- I think this WIP patch generates the code that we want, for 32 bit and 64. In the 64-bit backends, the 8 byte memcpy is expandend to just a load and store (see also comments below about the backend dealing with the memcpy).  So what I said earlier, that we also generate the libcall for X86 and AArch64 with -Oz, this wasn't true due to a problem in my test.
- But the main problem now is that we are not happy with the current implementation.

Just checking, and I think we also agree on this, we are saying the same things here:

> If we want to distinguish optimizing for size from optimizing for speed, that belongs in the backend - and it's already there. For example, see SelectionDAG::getMemcpy() and MemCpyOptimizer.cpp.

Exactly, so we rewrite the library call too early here in InstCombine, and the backend should deal with it, which is what this patch was trying to achieve.

Looks like my approach is going to be this then:

- first a NFC patch to clean up this existing bit of code using datalayout,
- then I will try to expand on this.


https://reviews.llvm.org/D52081





More information about the llvm-commits mailing list