[PATCH] D35035: [InstCombine] Prevent memcpy generation for small data size

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D35035#1252975, @spatel wrote:

> In https://reviews.llvm.org/D35035#1252829, @hiraditya wrote:
>
> > In https://reviews.llvm.org/D35035#1252796, @SjoerdMeijer wrote:
> >
> > > Perhaps the impact is negligible, non-existent, and we worry about this for nothing. As also suggested earlier, I will try to get some numbers on the table for ARM and AArch64 if we strip out the lowering here, if that is helpful for this discussion, but probably need a day or two to get them.
> >
> >
> > If you could provide some numbers, I can go ahead and remove the inlining of memcpy altogether provided the reviewers agree with it, or we can merge this patch which is trying to improve on previously hardcoded numbers.
>
>
> Yes, I support removing the expansion entirely, but I don't think we can commit that change without doing some advance perf testing.
>  And yes, in the best case, we'll discover that there are no regressions because all of the other analyses and lowering will do the transform as intended when it's profitable.


Just to drop a nit, here is a somewhat idiomatic pattern, that is recommended to avoid breaking strict aliasing: https://godbolt.org/z/_JNwOp
If we don't do this `memcpy` expansion where, will we have `memcpy` till the backend?
Surely this is not good.


https://reviews.llvm.org/D35035