Revert "[Intrinsics][PreISelInstrinsicLowering] llvm.memcpy.inline length no longer needs to be constant (#98281)"

This reverts commit 522fd53 while
unexpected mlir failures are investigated and resolved.

Author: asb

llvm/docs/LangRef.rst

@@ -15026,7 +15026,7 @@ Arguments:
 """"""""""
 
 The first argument is a pointer to the destination, the second is a
-pointer to the source. The third argument is an integer argument
+pointer to the source. The third argument is a constant integer argument
 specifying the number of bytes to copy, and the fourth is a
 boolean indicating a volatile access.
 

llvm/include/llvm/IR/IntrinsicInst.h

@@ -1296,6 +1296,9 @@ class MemMoveInst : public MemTransferInst {
 /// This class wraps the llvm.memcpy.inline intrinsic.
 class MemCpyInlineInst : public MemCpyInst {
 public:
+  ConstantInt *getLength() const {
+    return cast<ConstantInt>(MemCpyInst::getLength());
+  }
   // Methods for support type inquiry through isa, cast, and dyn_cast:
   static bool classof(const IntrinsicInst *I) {
     return I->getIntrinsicID() == Intrinsic::memcpy_inline;

llvm/include/llvm/IR/Intrinsics.td

@@ -966,14 +966,15 @@ def int_memcpy  : Intrinsic<[],
 // Memcpy semantic that is guaranteed to be inlined.
 // In particular this means that the generated code is not allowed to call any
 // external function.
+// The third argument (specifying the size) must be a constant.
 def int_memcpy_inline
     : Intrinsic<[],
       [llvm_anyptr_ty, llvm_anyptr_ty, llvm_anyint_ty, llvm_i1_ty],
       [IntrArgMemOnly, IntrWillReturn, IntrNoFree, IntrNoCallback,
        NoCapture<ArgIndex<0>>, NoCapture<ArgIndex<1>>,
        NoAlias<ArgIndex<0>>, NoAlias<ArgIndex<1>>,
        WriteOnly<ArgIndex<0>>, ReadOnly<ArgIndex<1>>,
-       ImmArg<ArgIndex<3>>]>;
+       ImmArg<ArgIndex<2>>, ImmArg<ArgIndex<3>>]>;
 
 def int_memmove : Intrinsic<[],
                             [llvm_anyptr_ty, llvm_anyptr_ty, llvm_anyint_ty,

llvm/lib/Analysis/Lint.cpp

@@ -290,8 +290,7 @@ void Lint::visitCallBase(CallBase &I) {
 
       // TODO: Check more intrinsics
 
-    case Intrinsic::memcpy:
-    case Intrinsic::memcpy_inline: {
+    case Intrinsic::memcpy: {
       MemCpyInst *MCI = cast<MemCpyInst>(&I);
       visitMemoryReference(I, MemoryLocation::getForDest(MCI),
                            MCI->getDestAlign(), nullptr, MemRef::Write);
@@ -312,6 +311,23 @@ void Lint::visitCallBase(CallBase &I) {
             "Undefined behavior: memcpy source and destination overlap", &I);
       break;
     }
+    case Intrinsic::memcpy_inline: {
+      MemCpyInlineInst *MCII = cast<MemCpyInlineInst>(&I);
+      const uint64_t Size = MCII->getLength()->getValue().getLimitedValue();
+      visitMemoryReference(I, MemoryLocation::getForDest(MCII),
+                           MCII->getDestAlign(), nullptr, MemRef::Write);
+      visitMemoryReference(I, MemoryLocation::getForSource(MCII),
+                           MCII->getSourceAlign(), nullptr, MemRef::Read);
+
+      // Check that the memcpy arguments don't overlap. The AliasAnalysis API
+      // isn't expressive enough for what we really want to do. Known partial
+      // overlap is not distinguished from the case where nothing is known.
+      const LocationSize LS = LocationSize::precise(Size);
+      Check(AA->alias(MCII->getSource(), LS, MCII->getDest(), LS) !=
+                AliasResult::MustAlias,
+            "Undefined behavior: memcpy source and destination overlap", &I);
+      break;
+    }
     case Intrinsic::memmove: {
       MemMoveInst *MMI = cast<MemMoveInst>(&I);
       visitMemoryReference(I, MemoryLocation::getForDest(MMI),

llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp

@@ -230,21 +230,6 @@ bool PreISelIntrinsicLowering::expandMemIntrinsicUses(Function &F) const {
 
       break;
     }
-    case Intrinsic::memcpy_inline: {
-      // Only expand llvm.memcpy.inline with non-constant length in this
-      // codepath, leaving the current SelectionDAG expansion for constant
-      // length memcpy intrinsics undisturbed.
-      auto *Memcpy = cast<MemCpyInlineInst>(Inst);
-      if (isa<ConstantInt>(Memcpy->getLength()))
-        break;
-
-      Function *ParentFunc = Memcpy->getFunction();
-      const TargetTransformInfo &TTI = LookupTTI(*ParentFunc);
-      expandMemCpyAsLoop(Memcpy, TTI);
-      Changed = true;
-      Memcpy->eraseFromParent();
-      break;
-    }
     case Intrinsic::memmove: {
       auto *Memmove = cast<MemMoveInst>(Inst);
       Function *ParentFunc = Memmove->getFunction();
@@ -306,7 +291,6 @@ bool PreISelIntrinsicLowering::lowerIntrinsics(Module &M) const {
     default:
       break;
     case Intrinsic::memcpy:
-    case Intrinsic::memcpy_inline:
     case Intrinsic::memmove:
     case Intrinsic::memset:
     case Intrinsic::memset_inline:

llvm/test/Transforms/PreISelIntrinsicLowering/X86/memcpy-inline-non-constant-len.ll

@@ -36,6 +36,14 @@ define void @memcpy_inline_is_volatile(ptr %dest, ptr %src, i1 %is.volatile) {
   ret void
 }
 
+define void @memcpy_inline_variable_size(ptr %dest, ptr %src, i32 %size) {
+  ; CHECK: immarg operand has non-immediate parameter
+  ; CHECK-NEXT: i32 %size
+  ; CHECK-NEXT: call void @llvm.memcpy.inline.p0.p0.i32(ptr %dest, ptr %src, i32 %size, i1 true)
+  call void @llvm.memcpy.inline.p0.p0.i32(ptr %dest, ptr %src, i32 %size, i1 true)
+  ret void
+}
+
 declare void @llvm.memmove.p0.p0.i32(ptr nocapture, ptr nocapture, i32, i1)
 define void @memmove(ptr %dest, ptr %src, i1 %is.volatile) {
   ; CHECK: immarg operand has non-immediate parameter