[PATCH] D117095: [BasicAA] Add support for memmove intrinsic

Mon Jan 24 04:39:58 PST 2022

ebrevnov added a comment.

In D117095#3243989 <https://reviews.llvm.org/D117095#3243989>, @reames wrote:

> @ebrevnov Thanks for the explanation, that makes a lot more sense!
>
> Your explanation raises a question for me.  Have we ever defined the semantics of operand bundles on intrinsic calls?  We've certainly done so for arbitrary calls, but intrinsics are somewhat "special".  The whole point of them is that they have known semantics.  This is purely a "I don't remember" question, the answer may be obvious.  I'm just hoping for  reminder/pointer to something to spark my memory.

Don't know either. As I can judge from common logic and implementation of "CallBase::hasReadingOperandBundles()" (especially comment) the same rules should be applied to intrinsics.

> On the solution, I'm really hesitant to generalize the modref behavior for this case.  That's a bunch of complexity for one narrow example.  I think we need a couple more motivating examples before that becomes reasonable.

That might be pretty general problem for us as virtually any call has deopt bundle. I'm gathering statistics to better understand the scope. Will share once available.

> On the other hand, patching the memcpy special case to be correct with operand bundles, and then applying this patch so it applies to memmove too would seem reasonable.  Particularly since we can now include a nice comment about about why this is not handled by more generic code elsewhere.  :)

Agree, it's not clear how fast if ever we can support that on general matters. Fixed the comment and rebased on top of D118033 <https://reviews.llvm.org/D118033>

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117095