[PATCH] D86066: IR: Merge MemCpyInlineInst and MemCpyInst

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet added a subscriber: jfb.
gchatelet added a comment.

In D86066#2221190 <https://reviews.llvm.org/D86066#2221190>, @arsenm wrote:

> Still requires updating more places that create new memcpys from existing ones to clean up this mess

Originally `llvm.memcpy.inline` has been created as a separate IR instruction to allow for the semantic to diverge if needed.
Now in the light of D79279 <https://reviews.llvm.org/D79279> it seems that we're heading towards many subtlety different IR memory functions, this comes with maintenance cost and complexity.

Maybe we should keep only one that has all the expressive power and lower to it.

I'm summing up the envisioned properties for `memcpy` here (mostly the ones from D79279 <https://reviews.llvm.org/D79279>)

- Source and destination pointers:
  - must be of any trivially copyable types
  - may come with alignment guarantees
  - may be tagged as `volatile` or provide atomic access semantic
  - may be from different pointer spaces? <- not sure about this one
- A constant `force_inline` argument can instruct the compiler to generate the whole content inline, in that case `size` must be a `ConstantInt`, if this argument is `false`, the compiler may choose to delegate implementation to an external function that has the same semantic (libc or user provided).

The same exercise would be needed for `memset` and `memcmp`.

@jfb what do you think? I know your patch is almost ready so I'd rather have this discussion before it's submitted.
Maybe this conversation should take place on the dev list even?


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

https://reviews.llvm.org/D86066