[SimplifyLibCalls][NFC] Clean up LibCallSimplifier from 'memset + malloc into calloc' transformation

[yurai007]

Summary

FoldMallocMemset can be safely removed because since https://reviews.llvm.org/D103009
such transformation is already performed in DSE.

[yurai007]

TODO: rebase when D103009 is delivered.

[yurai007]

Rebased to D103009.

[nikic]

llvm/test/Transforms/DeadStoreElimination/noop-stores.ll

350 ↗

(On Diff #349246)

Better include this in the other change.

llvm/test/Transforms/InstCombine/memset-1.ll

32

You can leave the tests here and add a comment that this pattern is now handled by DSE in a more general way.

[yurai007]

Rebased and addressed comments.

[yurai007]

@nikic: Kindly ping, all comments were addressed.

[yurai007]

Ping^2

[nikic]

LG

[gAlfonso-bit]

Can we cherry pick this to release-13.0/12 too since it also has the change that made the removed code in this patch obsolete in the first place?

[yurai007]

Rebased.

llvm/include/llvm/Transforms/Utils/SimplifyLibCalls.h

private:
   void eraseFromParent(Instruction *I);
 
   /// Replace an instruction with a value and erase it from its parent.
   void substituteInParent(Instruction *I, Value *With) {
     replaceAllUsesWith(I, With);
     eraseFromParent(I);
   }
 
-  Value *foldMallocMemset(CallInst *Memset, IRBuilderBase &B);
-
 public:
   LibCallSimplifier(
       const DataLayout &DL, const TargetLibraryInfo *TLI,
       OptimizationRemarkEmitter &ORE,
       BlockFrequencyInfo *BFI, ProfileSummaryInfo *PSI,
       function_ref<void(Instruction *, Value *)> Replacer =
           &replaceAllUsesWithDefault,
       function_ref<void(Instruction *)> Eraser = &eraseFromParentDefault);

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

Value *LibCallSimplifier::optimizeMemMove(CallInst *CI, IRBuilderBase &B) {
   CallInst *NewCI = B.CreateMemMove(CI->getArgOperand(0), Align(1),
                                     CI->getArgOperand(1), Align(1), Size);
   NewCI->setAttributes(CI->getAttributes());
   NewCI->removeAttributes(AttributeList::ReturnIndex,
                           AttributeFuncs::typeIncompatible(NewCI->getType()));
   return CI->getArgOperand(0);
 }
 
-/// Fold memset[_chk](malloc(n), 0, n) --> calloc(1, n).
-Value *LibCallSimplifier::foldMallocMemset(CallInst *Memset, IRBuilderBase &B) {
-  // This has to be a memset of zeros (bzero).
-  auto *FillValue = dyn_cast<ConstantInt>(Memset->getArgOperand(1));
-  if (!FillValue || FillValue->getZExtValue() != 0)
-    return nullptr;
-
-  // TODO: We should handle the case where the malloc has more than one use.
-  // This is necessary to optimize common patterns such as when the result of
-  // the malloc is checked against null or when a memset intrinsic is used in
-  // place of a memset library call.
-  auto *Malloc = dyn_cast<CallInst>(Memset->getArgOperand(0));
-  if (!Malloc || !Malloc->hasOneUse())
-    return nullptr;
-
-  // Is the inner call really malloc()?
-  Function *InnerCallee = Malloc->getCalledFunction();
-  if (!InnerCallee)
-    return nullptr;
-
-  LibFunc Func;
-  if (!TLI->getLibFunc(*InnerCallee, Func) || !TLI->has(Func) ||
-      Func != LibFunc_malloc)
-    return nullptr;
-
-  // The memset must cover the same number of bytes that are malloc'd.
-  if (Memset->getArgOperand(2) != Malloc->getArgOperand(0))
-    return nullptr;
-
-  // Replace the malloc with a calloc. We need the data layout to know what the
-  // actual size of a 'size_t' parameter is.
-  B.SetInsertPoint(Malloc->getParent(), ++Malloc->getIterator());
-  const DataLayout &DL = Malloc->getModule()->getDataLayout();
-  IntegerType *SizeType = DL.getIntPtrType(B.GetInsertBlock()->getContext());
-  if (Value *Calloc = emitCalloc(ConstantInt::get(SizeType, 1),
-                                 Malloc->getArgOperand(0),
-                                 Malloc->getAttributes(), B, *TLI)) {
-    substituteInParent(Malloc, Calloc);
-    return Calloc;
-  }
-
-  return nullptr;
-}
-
 Value *LibCallSimplifier::optimizeMemSet(CallInst *CI, IRBuilderBase &B) {
   Value *Size = CI->getArgOperand(2);
   annotateNonNullAndDereferenceable(CI, 0, Size, DL);
   if (isa<IntrinsicInst>(CI))
     return nullptr;
 
-  if (auto *Calloc = foldMallocMemset(CI, B))
-    return Calloc;
-
   // memset(p, v, n) -> llvm.memset(align 1 p, v, n)
   Value *Val = B.CreateIntCast(CI->getArgOperand(1), B.getInt8Ty(), false);
   CallInst *NewCI = B.CreateMemSet(CI->getArgOperand(0), Val, Size, Align(1));
   NewCI->setAttributes(CI->getAttributes());
   NewCI->removeAttributes(AttributeList::ReturnIndex,
                           AttributeFuncs::typeIncompatible(NewCI->getType()));
   return CI->getArgOperand(0);
 }
if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(CI)) {
     case Intrinsic::exp2:
       return optimizeExp2(CI, Builder);
     case Intrinsic::log:
     case Intrinsic::log2:
     case Intrinsic::log10:
       return optimizeLog(CI, Builder);
     case Intrinsic::sqrt:
       return optimizeSqrt(CI, Builder);
-    // TODO: Use foldMallocMemset() with memset intrinsic.
     case Intrinsic::memset:
       return optimizeMemSet(CI, Builder);
     case Intrinsic::memcpy:
       return optimizeMemCpy(CI, Builder);
     case Intrinsic::memmove:
       return optimizeMemMove(CI, Builder);
     default:
       return nullptr;
NewCI->removeAttributes(AttributeList::ReturnIndex,
                             AttributeFuncs::typeIncompatible(NewCI->getType()));
     return CI->getArgOperand(0);
   }
   return nullptr;
 }
 
 Value *FortifiedLibCallSimplifier::optimizeMemSetChk(CallInst *CI,
                                                      IRBuilderBase &B) {
-  // TODO: Try foldMallocMemset() here.
-
   if (isFortifiedCallFoldable(CI, 3, 2)) {
     Value *Val = B.CreateIntCast(CI->getArgOperand(1), B.getInt8Ty(), false);
     CallInst *NewCI = B.CreateMemSet(CI->getArgOperand(0), Val,
                                      CI->getArgOperand(2), Align(1));
     NewCI->setAttributes(CI->getAttributes());
     NewCI->removeAttributes(AttributeList::ReturnIndex,
                             AttributeFuncs::typeIncompatible(NewCI->getType()));
     return CI->getArgOperand(0);

llvm/test/Transforms/InstCombine/memset-1.ll

 ; CHECK-NEXT:    [[TMP1:%.*]] = trunc i32 [[VAL:%.*]] to i8
 ; CHECK-NEXT:    call void @llvm.memset.p0i8.i32(i8* align 1 [[MEM:%.*]], i8 [[TMP1]], i32 [[SIZE:%.*]], i1 false)
 ; CHECK-NEXT:    ret i8* [[MEM]]
 ;
   %ret = call i8* @memset(i8* %mem, i32 %val, i32 %size)
   ret i8* %ret
 }
 
+; Malloc + memset pattern is now handled by DSE in a more general way.
+
 define i8* @pr25892_lite(i32 %size) #0 {
 ; CHECK-LABEL: @pr25892_lite(
-; CHECK-NEXT:    [[CALLOC:%.*]] = call i8* @calloc(i32 1, i32 [[SIZE:%.*]])
-; CHECK-NEXT:    ret i8* [[CALLOC]]
+; CHECK-NEXT:    [[CALL:%.*]] = call i8* @malloc(i32 [[SIZE:%.*]])
+; CHECK-NEXT:    call void @llvm.memset.p0i8.i32(i8* align 1 [[CALL]], i8 0, i32 [[SIZE]], i1 false)
 ;
   %call1 = call i8* @malloc(i32 %size) #1
   %call2 = call i8* @memset(i8* %call1, i32 0, i32 %size) #1
   ret i8* %call2
 }
 
-; FIXME: A memset intrinsic should be handled similarly to a memset() libcall.
+; A memset intrinsic should be handled similarly to a memset() libcall.
+; Notice that malloc + memset pattern is now handled by DSE in a more general way.
 
 define i8* @malloc_and_memset_intrinsic(i32 %n) #0 {
 ; CHECK-LABEL: @malloc_and_memset_intrinsic(
 ; CHECK-NEXT:    [[CALL:%.*]] = call i8* @malloc(i32 [[N:%.*]])
 ; CHECK-NEXT:    call void @llvm.memset.p0i8.i32(i8* align 1 [[CALL]], i8 0, i32 [[N]], i1 false)
 ; CHECK-NEXT:    ret i8* [[CALL]]
 ;
   %call = call i8* @malloc(i32 %n)
   call void @llvm.memset.p0i8.i32(i8* %call, i8 0, i32 %n, i32 1, i1 false)
   ret i8* %call
 }
 
 ; This should not create a calloc and should not crash the compiler.
+; Notice that malloc + memset pattern is now handled by DSE in a more general way.
 
 define i8* @notmalloc_memset(i32 %size, i8*(i32)* %notmalloc) {
 ; CHECK-LABEL: @notmalloc_memset(
 ; CHECK-NEXT:    [[CALL1:%.*]] = call i8* [[NOTMALLOC:%.*]](i32 [[SIZE:%.*]]) [[ATTR0:#.*]]
 ; CHECK-NEXT:    call void @llvm.memset.p0i8.i32(i8* align 1 [[CALL1]], i8 0, i32 [[SIZE]], i1 false) [[ATTR0]]
 ; CHECK-NEXT:    ret i8* [[CALL1]]
 ;
   %call1 = call i8* %notmalloc(i32 %size) #1
   %call2 = call i8* @memset(i8* %call1, i32 0, i32 %size) #1
   ret i8* %call2
 }
 
-; FIXME: memset(malloc(x), 0, x) -> calloc(1, x)
 ; This doesn't fire currently because the malloc has more than one use.
+; Notice that malloc + memset pattern is now handled by DSE in a more general way.
 
 define float* @pr25892(i32 %size) #0 {
 ; CHECK-LABEL: @pr25892(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[CALL:%.*]] = tail call i8* @malloc(i32 [[SIZE:%.*]]) [[ATTR0]]
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i8* [[CALL]], null
 ; CHECK-NEXT:    br i1 [[CMP]], label [[CLEANUP:%.*]], label [[IF_END:%.*]]
 ; CHECK:       if.end: