[MemCpyOpt] Use memcpy source directly if dest is known to be immutable from attributes

[khei4]

Summary

This patch aims to remove memcpy on immutable arguments from attributes.

We can safely remove src-to-dest memcpy dest is used as only the call parameter if following conditions are satisfied

This is called on memcpy dest pointer arguments attributed as immutable
during call. Try to use memcpy source directly if all of the following
conditions are satisfied.

1. The memcpy dst is neither modified during the call nor captured by the call. (if readonly, noalias, nocapture attributes on call-site.)
2. The memcpy dst is an alloca with known alignment & size.
   1. The memcpy length == the alloca size which ensures that the new pointer is dereferenceable for the required range
   2. The src pointer has alignment >= the alloca alignment or can be enforced so.
3. The memcpy dst and src is not modified between the memcpy and the call. (if MSSA clobber check is safe.)
4. The memcpy src is not modified during the call. (ModRef check shows no Mod.)

(from https://reviews.llvm.org/D150967#4359222 and https://reviews.llvm.org/D150967#4365466 )

precommit tests: https://reviews.llvm.org/D150967
Motivated to resolve https://github.com/rust-lang/rust/issues/107436

[khei4]

update (currently attribute checking not working correctly

[nikic]

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

1750

This whole can can be replaced with CB->onlyReadsMemory(i). This will check readonly and readnone and will also correctly handle inheritance of attributes from the function.

1755

CB->paramHasAttr(i, Kind) is enough, it will handle attribute inheritance.

[khei4]

apply feedbacks, remove (most of) debug streams.

[khei4]

@nikic Thank you for the review! They saved me!

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

1755

Thank you for saving me! I haven't found the attribute inheritance behavior and am confused about that!

[nikic]

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

1737

This check looks unnecessary.

1738–1743

This seems simpler than the loop :)

[khei4]

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

1737

Thanks! That's remaining for the past diff!

1738–1743

Yeah, agreed, conditions are simpler than before.

[khei4]

attach Differential Revision on comment

[khei4]

process alignment, handle unescaped alloca and arg with noalias, clobber check on Dest of MDep(MemCpy)

[khei4]

need to change test on https://reviews.llvm.org/D150967#4365466 and refine description

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

1704–1705

dest check might be redundant because the first clobber check might already show dest is not modified between call and memcpy.

[khei4]

rebase

[khei4]

apply feedback on call with @nikic

1. remove noalias argument or unescaped alloca checks
2. add ModRefCheck for memcpy source

[nikic]

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

1627

nit: Start comments with uppercase letter.

1640

nit: auto * when using dyn_cast.

1661

see -> check that

1663

ImmutArg->stripPointerCasts() is same as AI.

1666

Outdated comment?

1667

byval -> alloca?

1678

Is the explicit MaybeAlign() here needed?

1693

"not modified by the call" maybe?

1740

nit: Unnecessary braces.

[nikic]

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

1695

MSSA->getMemoryAccess(&CB) is CallAccess.

1697

This second writtenBetween looks redundant to me. This is already ensured by getClobberingMemoryAccess().

1698

Hm, shouldn't this be checking the source? We already know the dest is not modified via readonly noalias.

1708

You can drop the bitcast code, not needed with opaque pointers.

[khei4]

apply feedbacks

[khei4]

@nikic
Thanks for the review! Too much obsolete comments!

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

1636–1642

I decided to postpone to the next Revision to collect all possible pointers and check that all ptr are alloca. (it might be a little conservative.)

1693

"not modified by the call" maybe?

Literally right,(writtenBetween guarantees so). TBH, I want to ensure the memcpy source is not modified by another alias during the call.
That part may be done by isModSet I guess.

1695

Good catch!! The same goes for ByVal.

1698

Right! Thanks, I was confused with them.

1708

Thanks! I believe ByVal has it too! Including other comments I'll fix them on ByVal side also!

[khei4]

fix typo

[khei4]

rebase

[khei4]

apply feedbacks.

• fix styles

[khei4]

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

1678

That was just to adjust arg types to the off-the-shelf util functions! Although it was wrong and removed!

[khei4]

fix styles and comments and postpone detailed case

[nikic]

It looks like the alignment checks are inverted.

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

1624

<= should be >= here?

1639

"Even if the arg can be based on multiple allocas" or so.

1647

&& !AllocaSize->isScalable() (and a test for this)

1662

"equals to" -> "equals the" or "is equal to the"

1673

Does that work? getZextValue() may assert for wide integers (though probably not really relevant here).

1676

smaller -> larger than or equal

1679

Then you don't need to check !MemDepAlign

1681–1683

1704

stray byval

llvm/test/Transforms/MemCpyOpt/memcpy.ll

455–456

This is not a VLA. You need to do something like alloca i8, i64 %n.

486–487

This case is fine, the problematic case is if the %val alignment is smaller.

[khei4]

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

1624

Right!

1647

&& !AllocaSize->isScalable()

Sorry, I'm not sure about a scalable vectors pointer could be transformed (maybe ||, not &&?). Do you mean scalable vectors TypeSize are available, but memcpy length cannot be scalable for it, so we couldn't handle it, right?

1673

Works fine! Thanks!

1679

Thank you!

llvm/test/Transforms/MemCpyOpt/memcpy.ll

486–487

Yeah, you seem right. Thank you for the comment!
I also found the alive2 has similar issues :)

https://alive2.llvm.org/ce/z/NZivTQ

[khei4]

apply feedback and inverse alignment vs memcpy length comparison.

also rebase for

• smaller alignment failure case
• address space failure case
• scalable vector failure case

[nikic]

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

1639

the all -> all the

1647

Yes, exactly.

1664

Add test with volatile memcpy.

1682

Can drop the extra () here.

1705

byval -> immut

llvm/test/Transforms/MemCpyOpt/memcpy.ll

464–465

I'd just write alloca <vscale x 2 x i32> here. You're using a "homogeneous scalable aggregate" which also tests the right thing, but is a more advanced feature...

471

We also need a test where dst is modified between call and memcpy, unless I missed it.

486–487

This comment is outdated.

500–501

We're missing a test where the alignment can be enforced. For that you want to copy from another alloca with lower alignment, and the alignment will be increased by the transform.

[khei4]

rebase
remove redundancy,
fix wrong comments

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

1647

Understood. Thanks!

llvm/test/Transforms/MemCpyOpt/memcpy.ll

464–465

Thanks! Yeah as you said in this case, unnecessary complexity!

500–501

Sorry, I'm not sure how this could be. I now found that MemDepAlign is just memcpy's arguments alignment attributes value, so basically enforcing from parameter attributes works, but I couldn't find the way to enforce lower alignment actually!
(I might too naive, but I tried for example,

@g = global ptr null, align 4

define void @immut_param_enforced_alignment(ptr align 16 noalias %val) {
  %val1 = alloca i8, align 16
  store ptr %val, ptr @g
  %p = load ptr, ptr @g
  call void @llvm.memcpy.p0.p0.i64(ptr align 16 %val1, ptr %p, i64 1, i1 false)
  call void @f(ptr nocapture noalias readonly %val1)
  ret void
}

this can't enforce the alignment.)

[nikic]

llvm/test/Transforms/MemCpyOpt/memcpy.ll

500–501

I believe you need something like this:

define void @test() {
  %val = alloca i8, align 1
  store i32 42, ptr %val
  %val1 = alloca i8, align 4
  call void @llvm.memcpy.p0.p0.i64(ptr %val1, ptr %val, i64 1, i1 false)
  call void @f_full_readonly(ptr %val1)
  ret void
}

Here the align 1 on the first alloca will be changed to align 4 by the transform.

[khei4]

rebase for enforced alignment

[khei4]

llvm/test/Transforms/MemCpyOpt/memcpy.ll

500–501

Thank you! Now I see DataLayout specify the alignment for stored types!

[nikic]

llvm/test/Transforms/MemCpyOpt/memcpy.ll

500–501

Hm, so I expected that for this test case the transform would happen and change to align 4 here. Do you know why it doesn't?

[nikic]

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

1694

I believe these MSSA->getMemoryAccess(MDep) calls can be replaced by Clobber.

llvm/test/Transforms/MemCpyOpt/memcpy.ll

500–501

Hm, this does work when I test the patch locally, so maybe just outdated test result?

[khei4]

llvm/test/Transforms/MemCpyOpt/memcpy.ll

500–501

Hm, this does work when I test the patch locally, so maybe just outdated test result?

Yeah, Sorry... I have forgotten to build before the test update. I believe this works fine.

[khei4]

update tests

[nikic]

LGTM

[khei4]

rebase

llvm/include/llvm/Transforms/Scalar/MemCpyOptimizer.h

bool performCallSlotOptzn(Instruction *cpyLoad, Instruction *cpyStore,
                             std::function<CallInst *()> GetC);
   bool processMemCpyMemCpyDependence(MemCpyInst *M, MemCpyInst *MDep,
                                      BatchAAResults &BAA);
   bool processMemSetMemCpyDependence(MemCpyInst *MemCpy, MemSetInst *MemSet,
                                      BatchAAResults &BAA);
   bool performMemCpyToMemSetOptzn(MemCpyInst *MemCpy, MemSetInst *MemSet,
                                   BatchAAResults &BAA);
   bool processByValArgument(CallBase &CB, unsigned ArgNo);
+  bool processImmutArgument(CallBase &CB, unsigned ArgNo);
   Instruction *tryMergingIntoMemset(Instruction *I, Value *StartPtr,
                                     Value *ByteVal);
   bool moveUp(StoreInst *SI, Instruction *P, const LoadInst *LI);
 
   void eraseInstruction(Instruction *I);
   bool iterateOnFunction(Function &F);
 };
 
 } // end namespace llvm
 
 #endif // LLVM_TRANSFORMS_SCALAR_MEMCPYOPTIMIZER_H

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

LLVM_DEBUG(dbgs() << "MemCpyOptPass: Forwarding memcpy to byval:\n"
                     << "  " << CB << "\n");
 
   // Otherwise we're good!  Update the byval argument.
   CB.setArgOperand(ArgNo, TmpCast);
   ++NumMemCpyInstr;
   return true;
 }
 
+/// This is called on arguments attributed with immutable arguments
+/// 1. The memcpy dst is not modified, captured by the call. (noalias readonly
+/// nocapture on call-site.
+/// 2. the pointer passed to the memcpy dest is an alloca with known alignment &
+/// size.
+///     2-1. the memcpy size <= the alloca size which ensures that the new
+///     pointer is dereferenceable for the required range
+///     2-2. the src pointer has alignment >= the alloca alignment
+/// 3. The memcpy dst and src is not modified between the memcpy and the call.
+/// (if MSSA clobber check is safe.
+/// 4. The memcpy src is not modified during the call. (if src is attributed
+/// with `noalias` or src is unescaped alloca,
+bool MemCpyOptPass::processImmutArgument(CallBase &CB, unsigned ArgNo) {
+  // Ensure passed argument is immutable during call.
+  if (!(CB.paramHasAttr(ArgNo, Attribute::NoAlias) &&
+        CB.paramHasAttr(ArgNo, Attribute::NoCapture)))
+    return false;
+  const DataLayout &DL = CB.getCaller()->getParent()->getDataLayout();
+  Value *ImmutArg = CB.getArgOperand(ArgNo);
+
+  // Check that Arg is Alloca
+  // TODO: Even if the arg get back to branches, we can remove memcpy if the all
+  // alloca alignments can be enforced to source alignment.
+  AllocaInst *AI = dyn_cast<AllocaInst>(ImmutArg->stripPointerCasts());
+  if (!AI)
+    return false;
+
+  std::optional<TypeSize> AllocaSize = AI->getAllocationSize(DL);
+  // Can't handle unknown size alloca (e.g. VLA)
+  if (!AllocaSize)
+    return false;
+  MemoryLocation Loc(ImmutArg, LocationSize::precise(*AllocaSize));
+  MemoryUseOrDef *CallAccess = MSSA->getMemoryAccess(&CB);
+  if (!CallAccess)
+    return false;
+
+  MemCpyInst *MDep = nullptr;
+  BatchAAResults BAA(*AA);
+  MemoryAccess *Clobber = MSSA->getWalker()->getClobberingMemoryAccess(
+      CallAccess->getDefiningAccess(), Loc, BAA);
+  if (auto *MD = dyn_cast<MemoryDef>(Clobber))
+    MDep = dyn_cast_or_null<MemCpyInst>(MD->getMemoryInst());
+
+  // If the immut argument isn't fed by a memcpy, ignore it.  If it is fed by
+  // a memcpy, check thta the arg equals to memcpy dest.
+  if (!MDep || MDep->isVolatile() || AI != MDep->getDest())
+    return false;
+
+  // The address space of the memcpy source must match the immut argument
+  if (MDep->getSource()->getType()->getPointerAddressSpace() !=
+      ImmutArg->getType()->getPointerAddressSpace())
+    return false;
+
+  // The length of the memcpy must be equal to the size of the alloca.
+  // memcpy size == allocasize
+  auto *MDepLen = dyn_cast<ConstantInt>(MDep->getLength());
+  if (!MDepLen || AllocaSize != MDepLen->getValue().getZExtValue())
+    return false;
+
+  // If memcpy source align is greater than the alloca's align, then we check to
+  // see if we can force the source of the memcpy to the alignment we need.
+  // If we fail, we bail out.
+  MaybeAlign MemDepAlign = MDep->getSourceAlign();
+  Align AllocaAlign = AI->getAlign();
+  if ((!MemDepAlign || AllocaAlign < *MemDepAlign) &&
+      AllocaAlign < getOrEnforceKnownAlignment(MDep->getSource(), MemDepAlign,
+                                               DL, &CB, AC, DT))
+    return false;
+
+  // Verify that the source and dest doesn't change in between the memcpy and
+  // the call.
+  //    memcpy(a <- b)
+  //    *b = 42;
+  //    foo(*a)
+  // It would be invalid to transform the second memcpy into foo(*b).
+  if (writtenBetween(MSSA, BAA, MemoryLocation::getForSource(MDep),
+                     MSSA->getMemoryAccess(MDep), CallAccess) ||
+      isModSet(AA->getModRefInfo(&CB, MemoryLocation::getForSource(MDep))))
+    return false;
+
+  LLVM_DEBUG(dbgs() << "MemCpyOptPass: Forwarding memcpy to Immut src:\n"
+                    << "  " << *MDep << "\n"
+                    << "  " << CB << "\n");
+
+  // Otherwise we're good!  Update the byval argument.
+  CB.setArgOperand(ArgNo, MDep->getSource());
+  ++NumMemCpyInstr;
+  return true;
+}
+
 /// Executes one iteration of MemCpyOptPass.
 bool MemCpyOptPass::iterateOnFunction(Function &F) {
   bool MadeChange = false;
 
   // Walk all instruction in the function.
   for (BasicBlock &BB : F) {
     // Skip unreachable blocks. For example processStore assumes that an
     // instruction in a BB can't be dominated by a later instruction in the
     // same BB (which is a scenario that can happen for an unreachable BB that
     // has itself as a predecessor).
     if (!DT->isReachableFromEntry(&BB))
for (BasicBlock::iterator BI = BB.begin(), BE = BB.end(); BI != BE;) {
         MadeChange |= processStore(SI, BI);
       else if (auto *M = dyn_cast<MemSetInst>(I))
         RepeatInstruction = processMemSet(M, BI);
       else if (auto *M = dyn_cast<MemCpyInst>(I))
         RepeatInstruction = processMemCpy(M, BI);
       else if (auto *M = dyn_cast<MemMoveInst>(I))
         RepeatInstruction = processMemMove(M);
       else if (auto *CB = dyn_cast<CallBase>(I)) {
-        for (unsigned i = 0, e = CB->arg_size(); i != e; ++i)
+        for (unsigned i = 0, e = CB->arg_size(); i != e; ++i) {
           if (CB->isByValArgument(i))
             MadeChange |= processByValArgument(*CB, i);
+          // TODO: this can be also decided by MemSSA analysis
+          else if (CB->onlyReadsMemory(i)) {
+            MadeChange |= processImmutArgument(*CB, i);
+          }
+        }
       }
 
       // Reprocess the instruction if desired.
       if (RepeatInstruction) {
         if (BI != BB.begin())
           --BI;
         MadeChange = true;
       }
     }
   }
 
   return MadeChange;
 }
 
 PreservedAnalyses MemCpyOptPass::run(Function &F, FunctionAnalysisManager &AM) {
   auto &TLI = AM.getResult<TargetLibraryAnalysis>(F);
   auto *AA = &AM.getResult<AAManager>(F);
   auto *AC = &AM.getResult<AssumptionAnalysis>(F);
   auto *DT = &AM.getResult<DominatorTreeAnalysis>(F);
   auto *MSSA = &AM.getResult<MemorySSAAnalysis>(F);
 
   bool MadeChange = runImpl(F, &TLI, AA, AC, DT, &MSSA->getMSSA());

llvm/test/Transforms/MemCpyOpt/memcpy.ll

 declare void @f1(ptr nocapture sret(%struct.big))
 declare void @f2(ptr)
 
 declare void @f(ptr)
 declare void @f_full_readonly(ptr nocapture noalias readonly)
 declare void @f_full_readnone(ptr nocapture noalias readnone)
 declare void @f_full_memory_none(ptr nocapture noalias) memory(none)
 
-; TODO: remove memcpy, which is guaranteed to be invariant
-; before and after the call because of its attributes.
 define void @immut_param(ptr align 4 noalias %val) {
 ; CHECK-LABEL: @immut_param(
-; CHECK-NEXT:    [[VAL1:%.*]] = alloca i8, align 4
-; CHECK-NEXT:    call void @llvm.memcpy.p0.p0.i64(ptr align 4 [[VAL1]], ptr align 4 [[VAL:%.*]], i64 1, i1 false)
-; CHECK-NEXT:    call void @f(ptr noalias nocapture readonly align 4 [[VAL1]])
+; CHECK-NEXT:    call void @f(ptr noalias nocapture readonly align 4 [[VAL:%.*]])
 ; CHECK-NEXT:    ret void
 ;
   %val1 = alloca i8, align 4
   call void @llvm.memcpy.p0.p0.i64(ptr align 4 %val1, ptr align 4 %val, i64 1, i1 false)
   call void @f(ptr align 4 nocapture noalias readonly %val1)
   ret void
 }
 
-; TODO: remove memcpy
 define void @immut_param_readonly(ptr align 4 noalias %val) {
 ; CHECK-LABEL: @immut_param_readonly(
-; CHECK-NEXT:    [[VAL1:%.*]] = alloca i8, align 4
-; CHECK-NEXT:    call void @llvm.memcpy.p0.p0.i64(ptr align 4 [[VAL1]], ptr align 4 [[VAL:%.*]], i64 1, i1 false)
-; CHECK-NEXT:    call void @f_full_readonly(ptr align 4 [[VAL1]])
+; CHECK-NEXT:    call void @f_full_readonly(ptr align 4 [[VAL:%.*]])
 ; CHECK-NEXT:    ret void
 ;
   %val1 = alloca i8, align 4
   call void @llvm.memcpy.p0.p0.i64(ptr align 4 %val1, ptr align 4 %val, i64 1, i1 false)
   call void @f_full_readonly(ptr align 4 %val1)
   ret void
 }
 
-; TODO: remove memcpy
 define void @immut_param_readnone(ptr align 4 noalias %val) {
 ; CHECK-LABEL: @immut_param_readnone(
-; CHECK-NEXT:    [[VAL1:%.*]] = alloca i8, align 4
-; CHECK-NEXT:    call void @f_full_readnone(ptr align 4 [[VAL1]])
+; CHECK-NEXT:    call void @f_full_readnone(ptr align 4 [[VAL:%.*]])
 ; CHECK-NEXT:    ret void
 ;
   %val1 = alloca i8, align 4
   call void @llvm.memcpy.p0.p0.i64(ptr align 4 %val1, ptr align 4 %val, i64 1, i1 false)
   call void @f_full_readnone(ptr align 4 %val1)
   ret void
 }
 
-; TODO: remove memcpy
 define void @immut_param_memory_none(ptr align 4 noalias %val) {
 ; CHECK-LABEL: @immut_param_memory_none(
 ; CHECK-NEXT:    [[VAL1:%.*]] = alloca i8, align 4
 ; CHECK-NEXT:    call void @f_full_memory_none(ptr align 4 [[VAL1]])
 ; CHECK-NEXT:    ret void
 ;
   %val1 = alloca i8, align 4
   call void @llvm.memcpy.p0.p0.i64(ptr align 4 %val1, ptr align 4 %val, i64 1, i1 false)
   call void @f_full_memory_none(ptr align 4 %val1)
   ret void
 }
 
-; TODO: remove memcpy
 define void @immut_param_no_align(ptr align 4 noalias %val) {
 ; CHECK-LABEL: @immut_param_no_align(
-; CHECK-NEXT:    [[VAL1:%.*]] = alloca i8, align 4
-; CHECK-NEXT:    call void @llvm.memcpy.p0.p0.i64(ptr align 4 [[VAL1]], ptr align 4 [[VAL:%.*]], i64 1, i1 false)
-; CHECK-NEXT:    call void @f(ptr noalias nocapture readonly [[VAL1]])
+; CHECK-NEXT:    call void @f(ptr noalias nocapture readonly [[VAL:%.*]])
 ; CHECK-NEXT:    ret void
 ;
   %val1 = alloca i8, align 4
   call void @llvm.memcpy.p0.p0.i64(ptr align 4 %val1, ptr align 4 %val, i64 1, i1 false)
   call void @f(ptr nocapture noalias readonly %val1)
   ret void
 }
 
 ; TODO: remove smaller memcpy than alloca size, it's not safe for general pointer,
 ; but ok for unescaped alloca dest.
 define void @immut_param_smaller_size(ptr align 4 noalias %val) {
 ; CHECK-LABEL: @immut_param_smaller_size(
 ; CHECK-NEXT:    [[VAL1:%.*]] = alloca i32, align 4
 ; CHECK-NEXT:    call void @llvm.memcpy.p0.p0.i64(ptr align 4 [[VAL1]], ptr align 4 [[VAL:%.*]], i64 1, i1 false)
 ; CHECK-NEXT:    call void @f(ptr noalias nocapture readonly [[VAL1]])
 ; CHECK-NEXT:    ret void
 ;
   %val1 = alloca i32, align 4
   call void @llvm.memcpy.p0.p0.i64(ptr align 4 %val1, ptr align 4 %val, i64 1, i1 false)
   call void @f(ptr nocapture noalias readonly %val1)
   ret void
 }
 
-; can't remove memcpy because src is modified between call and memcpy
+; Can't remove memcpy because src is modified between call and memcpy
 define void @immut_param_modified(ptr align 4 noalias %val) {
 ; CHECK-LABEL: @immut_param_modified(
 ; CHECK-NEXT:    [[VAL1:%.*]] = alloca i8, align 4
 ; CHECK-NEXT:    call void @llvm.memcpy.p0.p0.i64(ptr align 4 [[VAL1]], ptr align 4 [[VAL:%.*]], i64 1, i1 false)
 ; CHECK-NEXT:    store i32 13, ptr [[VAL]], align 4
 ; CHECK-NEXT:    call void @f(ptr noalias nocapture readonly align 4 [[VAL1]])
 ; CHECK-NEXT:    ret void
 ;
   %val1 = alloca i8, align 4
   call void @llvm.memcpy.p0.p0.i64(ptr align 4 %val1, ptr align 4 %val, i64 1, i1 false)
   store i32 13, ptr %val
   call void @f(ptr nocapture align 4 noalias readonly %val1)
   ret void
 }
 
-; can't remove memcpy if we remove, the alignment assumption would break
-; can't remove memcpy if we remove, the alignment assumption would break
+; Can't remove memcpy if we remove, the alignment assumption would break
 define void @immut_param_bigger_align(ptr align 16 noalias %val) {
 ; CHECK-LABEL: @immut_param_bigger_align(
 ; CHECK-NEXT:    [[VAL1:%.*]] = alloca i8, align 4
 ; CHECK-NEXT:    call void @llvm.memcpy.p0.p0.i64(ptr align 4 [[VAL1]], ptr [[VAL:%.*]], i64 1, i1 false)
 ; CHECK-NEXT:    call void @f(ptr noalias nocapture readonly [[VAL1]])
 ; CHECK-NEXT:    ret void
 ;
   %val1 = alloca i8, align 4
   call void @llvm.memcpy.p0.p0.i64(ptr align 4 %val1, ptr %val, i64 1, i1 false)
   call void @f(ptr nocapture noalias readonly %val1)
   ret void
 }
 
-; can't remove memcpy, because if the %val directly passed to @f,
+; Can't remove memcpy, because if the %val directly passed to @f,
 ; alignment of ptr to f's argument will be different.
 define void @immut_invalid_align_branched(i1 %c, ptr noalias %val) {
 ; CHECK-LABEL: @immut_invalid_align_branched(
 ; CHECK-NEXT:    [[VAL1:%.*]] = alloca [4 x i8], align 4
 ; CHECK-NEXT:    [[VAL2:%.*]] = alloca [16 x i8], align 16
 ; CHECK-NEXT:    [[VAL3:%.*]] = select i1 [[C:%.*]], ptr [[VAL1]], ptr [[VAL2]]
 ; CHECK-NEXT:    call void @llvm.memcpy.p0.p0.i64(ptr align 4 [[VAL3]], ptr align 4 [[VAL:%.*]], i64 4, i1 false)
 ; CHECK-NEXT:    call void @f(ptr noalias nocapture readonly [[VAL3]])
 ; CHECK-NEXT:    ret void
 ;
   %val1 = alloca [4 x i8], align 4
   %val2 = alloca [16 x i8], align 16
   %val3 = select i1 %c, ptr %val1, ptr %val2
   call void @llvm.memcpy.p0.p0.i64(ptr align 4 %val3, ptr align 4 %val, i64 4, i1 false)
   call void @f(ptr nocapture noalias readonly %val3)
   ret void
 }
 
-; can't remove memcpy, because alias might modify the src.
+; Can't remove memcpy, because alias might modify the src.
 define void @immut_but_alias_src(i1 %c, ptr %val) {
 ; CHECK-LABEL: @immut_but_alias_src(
 ; CHECK-NEXT:    [[VAL1:%.*]] = alloca i8, align 4
 ; CHECK-NEXT:    call void @llvm.memcpy.p0.p0.i64(ptr align 4 [[VAL1]], ptr align 4 [[VAL:%.*]], i64 1, i1 false)
 ; CHECK-NEXT:    call void @f(ptr noalias nocapture readonly [[VAL1]])
 ; CHECK-NEXT:    ret void
 ;
   %val1 = alloca i8, align 4
   call void @llvm.memcpy.p0.p0.i64(ptr align 4 %val1, ptr align 4 %val, i64 1, i1 false)
   call void @f(ptr nocapture noalias readonly %val1)
   ret void
 }
 
-; TODO: remove memcpy
 define void @immut_unescaped_alloca(i1 %c) {
 ; CHECK-LABEL: @immut_unescaped_alloca(
 ; CHECK-NEXT:    [[VAL:%.*]] = alloca i8, align 4
 ; CHECK-NEXT:    store i32 42, ptr [[VAL]], align 4
-; CHECK-NEXT:    [[VAL1:%.*]] = alloca i8, align 4
-; CHECK-NEXT:    call void @llvm.memcpy.p0.p0.i64(ptr align 4 [[VAL1]], ptr align 4 [[VAL]], i64 1, i1 false)
-; CHECK-NEXT:    call void @f_full_readonly(ptr [[VAL1]])
+; CHECK-NEXT:    call void @f_full_readonly(ptr [[VAL]])
 ; CHECK-NEXT:    ret void
 ;
   %val = alloca i8, align 4
   store i32 42, ptr %val
   %val1 = alloca i8, align 4
   call void @llvm.memcpy.p0.p0.i64(ptr align 4 %val1, ptr align 4 %val, i64 1, i1 false)
   call void @f_full_readonly(ptr %val1)
   ret void
 }
 
-; can't remove memcpy, because alloca src is modified
+; Can't remove memcpy, because alloca src is modified
 define void @immut_unescaped_alloca_modified(i1 %c) {
 ; CHECK-LABEL: @immut_unescaped_alloca_modified(
 ; CHECK-NEXT:    [[VAL:%.*]] = alloca i8, align 4
 ; CHECK-NEXT:    store i32 42, ptr [[VAL]], align 4
 ; CHECK-NEXT:    [[VAL1:%.*]] = alloca i8, align 4
 ; CHECK-NEXT:    call void @llvm.memcpy.p0.p0.i64(ptr align 4 [[VAL1]], ptr align 4 [[VAL]], i64 1, i1 false)
 ; CHECK-NEXT:    call void @f_full_readonly(ptr [[VAL1]])
 ; CHECK-NEXT:    ret void