[MemCpyOpt] Don't shorten memset if destination observable through unwinding

[nikic]

Summary

MemCpyOpt can shorten a memset if it is later partially overwritten by a memcpy. It checks that the destination is not read in between, but we also need to make sure that the destination cannot be observed via unwinding.

As a side-note, at least for the constant size case this transform should really be performed by DSE, and it does have some support for this (isShortenableAtTheBeginning), but apparently doesn't catch the memset+memcpy combination.

[jdoerfert]

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

1061

If nounwind is set this is not necessary.

[nikic]

As a side-note, at least for the constant size case this transform should really be performed by DSE, and it does have some support for this (isShortenableAtTheBeginning), but apparently doesn't catch the memset+memcpy combination.

After looking into this a bit more, the problem is that I tried something like this

define void @test(i8* %src, i8* %dst, i8 %c) {
  call void @llvm.memset.p0i8.i64(i8* %dst, i8 %c, i64 64, i1 false)
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 32, i1 false)
  ret void
}

which doesn't work, while adding a noalias on %dst does work. As a result of D86815, the memcpy now counts as a read-clobber of %dst, as dst and src may be equal. And with that in mind, the MemCpyOpt transform isn't correct either since that change, unless we know that dst and src are NoAlias. cc @fhahn

[nikic]

Add fast path if containing function is nounwind.

[jdoerfert]

Makes sense to me.

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

bool MemCpyOptPass::processMemSetMemCpyDependence(MemCpyInst *MemCpy,
   if (DstDepInfo.getInst() != MemSet)
     return false;
 
   // Use the same i8* dest as the memcpy, killing the memset dest if different.
   Value *Dest = MemCpy->getRawDest();
   Value *DestSize = MemSet->getLength();
   Value *SrcSize = MemCpy->getLength();
 
+  // If the destination might be accessible by the caller, make sure we cannot
+  // unwind between the memset and the memcpy.
+  if (!isa<AllocaInst>(getUnderlyingObject(Dest))) {
+    for (const Instruction &I : make_range(MemSet->getIterator(),
+                                           MemCpy->getIterator())) {
+      if (I.mayThrow())
+        return false;
+    }
+  }
+
   // By default, create an unaligned memset.
   unsigned Align = 1;
   // If Dest is aligned, and SrcSize is constant, use the minimum alignment
   // of the sum.
   const unsigned DestAlign =
       std::max(MemSet->getDestAlignment(), MemCpy->getDestAlignment());
   if (DestAlign > 1)
     if (ConstantInt *SrcSizeC = dyn_cast<ConstantInt>(SrcSize))

llvm/test/Transforms/MemCpyOpt/memset-memcpy-redundant-memset.ll

;
   call void @llvm.memset.p0i8.i64(i8* %a0, i8 0, i64 16, i1 false)
   store i8 1, i8* %a1
   call void @llvm.memcpy.p0i8.p0i8.i64(i8* %a0, i8* %b, i64 8, i1 false)
   ret void
 }
 
 define void @test_throwing_call(i8* %src, i64 %src_size, i8* %dst, i64 %dst_size, i8 %c) {
 ; CHECK-LABEL: @test_throwing_call(
+; CHECK-NEXT:    call void @llvm.memset.p0i8.i64(i8* [[DST:%.*]], i8 [[C:%.*]], i64 [[DST_SIZE:%.*]], i1 false)
 ; CHECK-NEXT:    call void @call() [[ATTR2:#.*]]
-; CHECK-NEXT:    [[TMP1:%.*]] = icmp ule i64 [[DST_SIZE:%.*]], [[SRC_SIZE:%.*]]
-; CHECK-NEXT:    [[TMP2:%.*]] = sub i64 [[DST_SIZE]], [[SRC_SIZE]]
-; CHECK-NEXT:    [[TMP3:%.*]] = select i1 [[TMP1]], i64 0, i64 [[TMP2]]
-; CHECK-NEXT:    [[TMP4:%.*]] = getelementptr i8, i8* [[DST:%.*]], i64 [[SRC_SIZE]]
-; CHECK-NEXT:    call void @llvm.memset.p0i8.i64(i8* align 1 [[TMP4]], i8 [[C:%.*]], i64 [[TMP3]], i1 false)
-; CHECK-NEXT:    call void @llvm.memcpy.p0i8.p0i8.i64(i8* [[DST]], i8* [[SRC:%.*]], i64 [[SRC_SIZE]], i1 false)
+; CHECK-NEXT:    call void @llvm.memcpy.p0i8.p0i8.i64(i8* [[DST]], i8* [[SRC:%.*]], i64 [[SRC_SIZE:%.*]], i1 false)
 ; CHECK-NEXT:    ret void
 ;
   call void @llvm.memset.p0i8.i64(i8* %dst, i8 %c, i64 %dst_size, i1 false)
   call void @call() readnone
   call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 %src_size, i1 false)
   ret void
 }