[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

1771

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.

1776

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

1776

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

1758

This check looks unnecessary.

1759–1764

This seems simpler than the loop :)

[khei4]

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

1758

Thanks! That's remaining for the past diff!

1759–1764

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?

1761

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

434–435

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

452–453

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

452–453

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

434–435

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...

438–439

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

452–453

This comment is outdated.

480

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

434–435

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

480

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

480

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

480

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

[nikic]

llvm/test/Transforms/MemCpyOpt/memcpy.ll

477–478

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

477–478

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

[khei4]

llvm/test/Transforms/MemCpyOpt/memcpy.ll

477–478

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
+/// TODO: write doc comments about the conditions
+/// dest ptr is not modified between memcpy and call
+/// 1) The memcpy dst is not modified by the call.
+/// 2) The memcpy dst is not modified between the memcpy and the call.(if MSSA
+/// clobber check is safe
+///
+/// 3) The memcpy src is not modified between the
+/// memcpy and the call. ((if src is attributed with noalias. and MSSA clobber
+/// check for srcis safe. ModRef check. Or src is unescaped alloca.
+bool MemCpyOptPass::processImmutArgument(CallBase &CB, unsigned ArgNo) {
+  // ensure passed argument is immutable during call.
+  // FIXME: no write on call base can be guaranteed by ModRefCheck
+  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: collect possible allocas like
+  // %val1 = alloca [4 x i8], align 4
+  // %val2 = alloca [16 x i8], align 4
+  // %val3 = select i1 %c, ptr %val1, ptr %val2
+  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, see the arg equals to memcpy dest
+  if (!MDep || MDep->isVolatile() ||
+      ImmutArg->stripPointerCasts() != MDep->getDest())
+    return false;
+
+  // currently handle only `noalias` attributed argument  or unescaped
+  // alloca also which are ensured only intra-procedual Clobber check is
+  // sufficient.
+  // TODO: more general patterns
+  Value *Src = MDep->getSource()->stripPointerCasts();
+  if (auto *Arg = dyn_cast<Argument>(Src);
+      !((Arg && Arg->hasNoAliasAttr()) || isa<AllocaInst>(Src)))
+    return false;
+
+  // currently handle only `noalias` attributed argument  or unescaped
+  // The length of the memcpy must be larger or equal to the size of the byval.
+  // memcpy size == allocasize
+  auto *MDepLen = dyn_cast<ConstantInt>(MDep->getLength());
+  if (!MDepLen || AllocaSize != MDepLen->getValue().getZExtValue())
+    return false;
+
+  Align AllocaAlign = AI->getAlign();
+  // If it is greater than the memcpy, 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();
+  if ((!MemDepAlign || *MemDepAlign < AllocaAlign) &&
+      getOrEnforceKnownAlignment(MDep->getSource(), MaybeAlign(AllocaAlign), DL,
+                                 &CB, AC, DT) < AllocaAlign)
+    return false;
+
+  // The address space of the memcpy source must match the immut argument
+  if (MDep->getSource()->getType()->getPointerAddressSpace() !=
+      ImmutArg->getType()->getPointerAddressSpace())
+    return false;
+
+  // Verify that the src 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), MSSA->getMemoryAccess(&CB)) ||
+      writtenBetween(MSSA, BAA, MemoryLocation::getForDest(MDep),
+                     MSSA->getMemoryAccess(MDep), MSSA->getMemoryAccess(&CB)))
+    return false;
+
+  Value *TmpCast = MDep->getSource();
+  if (MDep->getSource()->getType() != ImmutArg->getType()) {
+    BitCastInst *TmpBitCast =
+        new BitCastInst(MDep->getSource(), ImmutArg->getType(), "tmpcast", &CB);
+    // Set the tmpcast's DebugLoc to MDep's
+    TmpBitCast->setDebugLoc(MDep->getDebugLoc());
+    TmpCast = TmpBitCast;
+  }
+
+  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, TmpCast);
+  ++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
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_param1(ptr align 4 noalias %val) {
 ; CHECK-LABEL: @immut_param1(
 ; CHECK-NEXT:  start:
-; 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
 ;
 start:
   %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_param2_readonly(ptr align 4 noalias %val) {
 ; CHECK-LABEL: @immut_param2_readonly(
 ; CHECK-NEXT:  start:
-; 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
 ;
 start:
   %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_param2_readnone(ptr align 4 noalias %val) {
 ; CHECK-LABEL: @immut_param2_readnone(
 ; CHECK-NEXT:  start:
-; 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
 ;
 start:
   %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_param3_memory_none(ptr align 4 noalias %val) {
 ; CHECK-LABEL: @immut_param3_memory_none(
 ; CHECK-NEXT:  start:
 ; CHECK-NEXT:    [[VAL1:%.*]] = alloca i8, align 4
 ; CHECK-NEXT:    call void @f_full_memory_none(ptr align 4 [[VAL1]])
 ; CHECK-NEXT:    ret void
 ;
 start:
   %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 because neither alignment attribute,
-; nor Datalayout S specifications is given.
 define void @immut_param4_no_align(ptr align 4 noalias %val) {
 ; CHECK-LABEL: @immut_param4_no_align(
 ; CHECK-NEXT:  start:
+; CHECK-NEXT:    call void @f_full_readonly(ptr [[VAL:%.*]])
+; CHECK-NEXT:    ret void
+;
+start:
+  %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 of %val's mutability
+define void @immut_param_alias_src(ptr align 4 %val) {
+; CHECK-LABEL: @immut_param_alias_src(
+; CHECK-NEXT:  start:
 ; 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
 ;
 start:
   %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 if the %val directly passed to @f,
 ; alignment of ptr to f's argument will be different.
 define void @immut_invalid_align(i1 %c, ptr noalias %val) {
 ; CHECK-LABEL: @immut_invalid_align(
 ; 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]]
;
   %val1 = alloca [4 x i8], align 4
   %val2 = alloca [16 x i8], align 4
   %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
 }
 
-; TODO: remove memcpy, copied size is less than or equal to the alloca size
+; can't remove memcpy, copied size is less than or equal to the alloca size
 define void @immut_valid_small_align(i1 %c, ptr noalias align 4 %val) {
 ; CHECK-LABEL: @immut_valid_small_align(
 ; CHECK-NEXT:    [[VAL1:%.*]] = alloca [4 x i8], align 4
 ; CHECK-NEXT:    [[VAL2:%.*]] = alloca [16 x i8], align 4
 ; 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 1, 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 4
   %val3 = select i1 %c, ptr %val1, ptr %val2
   call void @llvm.memcpy.p0.p0.i64(ptr align 4 %val3, ptr align 4 %val, i64 1, i1 false)
   call void @f(ptr nocapture noalias readonly %val3)
   ret void
 }