[PATCH] D104801: [MemCpyOpt] Enable memcpy optimizations unconditionally.

[Hendrik Greving via Phabricator via llvm-commits]

hgreving added a comment.

In D104801#2890876 <https://reviews.llvm.org/D104801#2890876>, @tra wrote:

> In D104801#2889283 <https://reviews.llvm.org/D104801#2889283>, @hgreving wrote:
>
>> Besides that this patch created the intrinsics llvm.memset which in our target was unsupported, it also created pointers that may be invalid on the target, e.g. i8 addrspace(X)* where addrspace(X) only supports vector memory. While this is probably all solvable, please consider adding TTI switches for targets to disable this optimization (e.g. for the intrinsics).
>
> ...and that brings us back to where this patch has started -- with a TTI knob to control whether it wants this pass enabled. :-/
>
> That said, I'm not quite sure that "this type/AS combination" is invalid is something you can expect all parts of LLVM to respect. While this pass may happen to tickle it, I can see other cases where the type of the pointer may change. I would think that dealing with the quirks of type support of particular target  would be something for legalizer to do.

I agree to above. But in practice, since this change is quite drastic, I would recommend adding some kind of control for targets not wanting this. Also a flat-rate knob to disable the pass altogether would be fine IMO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104801