[PATCH] D71710: [instrinsics] Add @llvm.memcpy.inline instrinsics

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

A couple more minor issues.

> I see, I looked it up a bit but it will take some time for me to understand the framework and where to add this information.
>  Can you provide me with pointers?

There's code in BasicAAResult::getModRefInfo, which calls MemoryLocation::getForSource().  I guess you get the implementation of getForSource() for free, by inheriting from MemTransferInst, but I'm not sure we actually call it with your current patch, because you didn't modify classof for AnyMemCpyInst.

There might be a few other places where we could check for llvm.memcpy.inline, but nothing really high-priority, I think.



================
Comment at: llvm/include/llvm/IR/IntrinsicInst.h:669
   //  and llvm.memset/memcpy/memmove
   class AnyMemIntrinsic : public MemIntrinsicBase<AnyMemIntrinsic> {
   public:
----------------
Should memcpy_inline count as an AnyMemIntrinsic/AnyMemTransferInst/AnyMemCpyInst?


================
Comment at: llvm/lib/Analysis/Lint.cpp:351
+      const APInt &APLen = MCII->getLength()->getValue();
+      assert(APLen.isIntN(64) && "@llvm.memcpy.inline size must be uint64");
+      const uint64_t Size = APLen.getZExtValue();
----------------
`isIntN(64)`?  Your LangRef documentation explicitly says it doesn't have to be an i64.

You can just drop the assertion, and use getLimitedValue() instead of getZExtValue().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71710