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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D52081#1236517, @SjoerdMeijer wrote:

> Thanks for the feedback and suggestions! Summarising where we are:
>
> - I think this WIP patch generates the code that we want


//we// ?
Do keep in mind that there is more than one backend, more than one target architecture.

> , 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