[Memcpy Loop Lowering] Insert loop basic block inbetween the split basic block.

[sfertile]

Summary

When creating the basic block for the loop body when lowering memcpy, place it between the 2 blocks split from the original basic-block.

[hfinkel]

When creating the basic block for the loop body when lowering memcpy, place it between the 2 blocks split from the original basic-block.

Why? Is this fixing a bug in the current code under some conditions, or preparing for some future changes?

[sfertile]

In D41197#955577, @hfinkel wrote:

When creating the basic block for the loop body when lowering memcpy, place it between the 2 blocks split from the original basic-block.

Why? Is this fixing a bug in the current code under some conditions, or preparing for some future changes?

Neither, I noticed the difference between the original implementation and the wide implementation when comparing the expansions. Id like to cleanup the new implementation to match as close to the old as possible as part of removing the old implementation.

[hfinkel]

In D41197#955711, @sfertile wrote:

In D41197#955577, @hfinkel wrote:

When creating the basic block for the loop body when lowering memcpy, place it between the 2 blocks split from the original basic-block.

Why? Is this fixing a bug in the current code under some conditions, or preparing for some future changes?

Neither, I noticed the difference between the original implementation and the wide implementation when comparing the expansions. Id like to cleanup the new implementation to match as close to the old as possible as part of removing the old implementation.

LGTM. Please say this in the commit message.

[sfertile]

committed in r320848.

lib/Transforms/Utils/LowerMemIntrinsics.cpp

void llvm::createMemCpyLoopUnknownSize(Instruction *InsertBefore,
   assert(ILengthType &&
          "expected size argument to memcpy to be an integer type!");
   ConstantInt *CILoopOpSize = ConstantInt::get(ILengthType, LoopOpSize);
   Value *RuntimeLoopCount = PLBuilder.CreateUDiv(CopyLen, CILoopOpSize);
   Value *RuntimeResidual = PLBuilder.CreateURem(CopyLen, CILoopOpSize);
   Value *RuntimeBytesCopied = PLBuilder.CreateSub(CopyLen, RuntimeResidual);
 
   BasicBlock *LoopBB =
-      BasicBlock::Create(Ctx, "loop-memcpy-expansion", ParentFunc, nullptr);
+      BasicBlock::Create(Ctx, "loop-memcpy-expansion", ParentFunc, PostLoopBB);
   IRBuilder<> LoopBuilder(LoopBB);
 
   PHINode *LoopIndex = LoopBuilder.CreatePHI(CopyLenType, 2, "loop-index");
   LoopIndex->addIncoming(ConstantInt::get(CopyLenType, 0U), PreLoopBB);
 
   Value *SrcGEP = LoopBuilder.CreateInBoundsGEP(LoopOpType, SrcAddr, LoopIndex);
   Value *Load = LoopBuilder.CreateLoad(SrcGEP, SrcIsVolatile);
   Value *DstGEP = LoopBuilder.CreateInBoundsGEP(LoopOpType, DstAddr, LoopIndex);
   LoopBuilder.CreateStore(Load, DstGEP, DstIsVolatile);
 
   Value *NewIndex =
       LoopBuilder.CreateAdd(LoopIndex, ConstantInt::get(CopyLenType, 1U));
   LoopIndex->addIncoming(NewIndex, LoopBB);
 
   Type *Int8Type = Type::getInt8Ty(Ctx);
   if (LoopOpType != Int8Type) {
     // Loop body for the residual copy.
     BasicBlock *ResLoopBB = BasicBlock::Create(Ctx, "loop-memcpy-residual",
-                                               PreLoopBB->getParent(), nullptr);
+                                               PreLoopBB->getParent(),
+                                               PostLoopBB);
     // Residual loop header.
     BasicBlock *ResHeaderBB = BasicBlock::Create(
         Ctx, "loop-memcpy-residual-header", PreLoopBB->getParent(), nullptr);
 
     // Need to update the pre-loop basic block to branch to the correct place.
     // branch to the main loop if the count is non-zero, branch to the residual
     // loop if the copy size is smaller then 1 iteration of the main loop but
     // non-zero and finally branch to after the residual loop if the memcpy

test/CodeGen/NVPTX/lower-aggr-copies.ll

 ; WIR:         %loop-index = phi i64 [ 0, %entry ], [ [[IndexInc:%[0-9]+]], %loop-memcpy-expansion ]
 ; WIR:         [[SrcGep:%[0-9]+]] = getelementptr inbounds i8, i8* %src, i64 %loop-index
 ; WIR:         [[Load:%[0-9]+]] = load i8, i8* [[SrcGep]]
 ; WIR:         [[DstGep:%[0-9]+]] = getelementptr inbounds i8, i8* %dst, i64 %loop-index
 ; WIR:         store i8 [[Load]], i8* [[DstGep]]
 ; WIR:         [[IndexInc]] = add i64 %loop-index, 1
 ; WIR:         [[Cond2:%[0-9]+]] = icmp ult i64 [[IndexInc]], [[LoopCount]]
 ; WIR:         br i1 [[Cond2]], label %loop-memcpy-expansion, label %post-loop-memcpy-expansion
+
+; WIR-LABEL:   post-loop-memcpy-expansion:
+; WIR:         ret i8* %dst
 }
 
 define i8* @memcpy_volatile_caller(i8* %dst, i8* %src, i64 %n) #0 {
 entry:
   tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 %n, i32 1, i1 true)
   ret i8* %dst
 
 ; IR-LABEL:   @memcpy_volatile_caller

 ; WIR:         %loop-index = phi i64 [ 0, %entry ], [ [[IndexInc:%[0-9]+]], %loop-memcpy-expansion ]
 ; WIR:         [[SrcGep:%[0-9]+]] = getelementptr inbounds i8, i8* %src, i64 %loop-index
 ; WIR:         [[Load:%[0-9]+]] = load volatile i8, i8* [[SrcGep]]
 ; WIR:         [[DstGep:%[0-9]+]] = getelementptr inbounds i8, i8* %dst, i64 %loop-index
 ; WIR:         store volatile i8 [[Load]], i8* [[DstGep]]
 ; WIR:         [[IndexInc]] = add i64 %loop-index, 1
 ; WIR:         [[Cond2:%[0-9]+]] = icmp ult i64 [[IndexInc]], [[LoopCount]]
 ; WIR:         br i1 [[Cond2]], label %loop-memcpy-expansion, label %post-loop-memcpy-expansion
+
+; WIR-LABEL:   post-loop-memcpy-expansion:
+; WIR:         ret i8* %dst
 }
 
 define i8* @memcpy_casting_caller(i32* %dst, i32* %src, i64 %n) #0 {
 entry:
   %0 = bitcast i32* %dst to i8*
   %1 = bitcast i32* %src to i8*
   tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %0, i8* %1, i64 %n, i32 1, i1 false)
   ret i8* %0