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

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

Needs a LangRef update.

It would be nice to do a bit more work so various optimizations handle llvm.memcpy.inline in a way that isn't completely conservative (for example, in alias analysis), but I guess that doesn't need to be in the initial patch.



================
Comment at: llvm/include/llvm/IR/IntrinsicInst.h:659
+      return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
+    }
+  };
----------------
Maybe hide the base class getLength() with a version that returns a ConstantInt instead?  You could use it, for example, in Lint.cpp.


================
Comment at: llvm/include/llvm/IR/Intrinsics.td:515
+
+// Memcpy semantic that is guaranteed to be inlined.
+// The third argument (specifying the size) must be a constant.
----------------
Maybe clarify what "inlined" means.  In this context, it should mean specifically that it doesn't call any external function, including memcpy.  (It would be difficult to reliably guarantee that it doesn't contain any call instructions; for example, we might outline a memcpy sequence, and that's probably fine.)


================
Comment at: llvm/test/CodeGen/X86/memcpy-inline.ll:12
+; CHECK-NEXT:    retq
+  tail call void @llvm.memcpy.inline.p0i8.p0i8.i64(i8* %a, i8* %b, i64 8, i1 0 )
+  ret void
----------------
It would be a good idea to have a testcase for a larger size that wouldn't normally be inlined.  (Also, maybe a test on another target that doesn't have a "memcpy" instruction.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71710