[SLC] Convert some strndup calls to strdup calls

[xbolva00]

Summary

Motivation:

• If we can fold it to strdup, we should (strndup does more things than strdup).
• Annotation mechanism. (Works for strdup well).

strdup and strndup are part of C 20 (currently posix fns), so we should optimize them.

[xbolva00]

Removed useless condition

[xbolva00]

lib/Transforms/InstCombine/InstCombineCalls.cpp

4226–4227

sight, this breaks things (eg. zstd build)

[jdoerfert]

lib/Transforms/InstCombine/InstCombineCalls.cpp

4226–4227

Why ;) ?

test/Transforms/InstCombine/strndup.ll

22

Why don't we get a dereferenceable_or_null(4) here?

[xbolva00]

test/Transforms/InstCombine/strndup.ll

22

annotateAllocSite doesnt know about strndup yet :) follow-up patch will solve it.

[jdoerfert]

optimizeStrNDup seems fine to me, the breakage that you mentioned need to be addressed though.

[xbolva00]

Fixed crash

[xbolva00]

lib/Transforms/InstCombine/InstCombineCalls.cpp

4226–4227

This function (visitAllocSite) is just weird. Looking at eraseInstFromFunction, eraseInstFromFunction always returns nullptr; so visitAllocSite returns nullptr in both branches too. If I changed it, it breaks the compiler on some tests.

Instead of hacking this, if think this new patch is reasonable. First, it tries to optimize call, and then possibly visit (remove/annotate) AllocSite.

[lebedev.ri]

I think some patches just severely lack words.
It is entirely non-obvious to me why this fold should happen, yet the patch description is empty.

[xbolva00]

Multiple reasons

1. If we can fold it to strdup, we should (strndup does more things than strdup).
2. Annotation mechanism. (Works for strdup well).

p.s: strdup and strndup are part of C 20 :)

[jdoerfert]

In D67679#1674025, @xbolva00 wrote:

Multiple reasons

1. If we can fold it to strdup, we should (strndup does more things than strdup).
2. Annotation mechanism. (Works for strdup well).

p.s: strdup and strndup are part of C 20 :)

Put this in the commit message. I missed that it was empty, thanks @lebedev.ri.

[xbolva00]

Added

[xbolva00]

@jdoerfert is this patch fine for you?

[jdoerfert]

LGTM.

include/llvm/Transforms/Utils/BuildLibCalls.h

namespace llvm {
   /// corresponding to \a Ty.
   StringRef getFloatFnName(const TargetLibraryInfo *TLI, Type *Ty,
                            LibFunc DoubleFn, LibFunc FloatFn,
                            LibFunc LongDoubleFn);
 
   /// Return V if it is an i8*, otherwise cast it to i8*.
   Value *castToCStr(Value *V, IRBuilder<> &B);
 
-  /// Emit a call to the strlen function to the builder, for the specified
+  /// Emit a call to the strdup function to the builder, for the specified
   /// pointer. Ptr is required to be some pointer type, and the return value has
   /// 'intptr_t' type.
   Value *emitStrLen(Value *Ptr, IRBuilder<> &B, const DataLayout &DL,
                     const TargetLibraryInfo *TLI);
 
+  /// Emit a call to the strlen function to the builder, for the specified
+  /// pointer. Ptr is required to be some pointer type, and the return value has
+  /// 'i8*' type.
+  Value *emitStrDup(Value *Ptr, IRBuilder<> &B, const TargetLibraryInfo *TLI);
+
   /// Emit a call to the strnlen function to the builder, for the specified
   /// pointer. Ptr is required to be some pointer type, MaxLen must be of size_t
   /// type, and the return value has 'intptr_t' type.
   Value *emitStrNLen(Value *Ptr, Value *MaxLen, IRBuilder<> &B,
                      const DataLayout &DL, const TargetLibraryInfo *TLI);
 
   /// Emit a call to the strchr function to the builder, for the specified
   /// pointer and character. Ptr is required to be some pointer type, and the

include/llvm/Transforms/Utils/SimplifyLibCalls.h

 private:
   // String and Memory Library Call Optimizations
   Value *optimizeStrCat(CallInst *CI, IRBuilder<> &B);
   Value *optimizeStrNCat(CallInst *CI, IRBuilder<> &B);
   Value *optimizeStrChr(CallInst *CI, IRBuilder<> &B);
   Value *optimizeStrRChr(CallInst *CI, IRBuilder<> &B);
   Value *optimizeStrCmp(CallInst *CI, IRBuilder<> &B);
   Value *optimizeStrNCmp(CallInst *CI, IRBuilder<> &B);
+  Value *optimizeStrNDup(CallInst *CI, IRBuilder<> &B);
   Value *optimizeStrCpy(CallInst *CI, IRBuilder<> &B);
   Value *optimizeStpCpy(CallInst *CI, IRBuilder<> &B);
   Value *optimizeStrNCpy(CallInst *CI, IRBuilder<> &B);
   Value *optimizeStrLen(CallInst *CI, IRBuilder<> &B);
   Value *optimizeStrPBrk(CallInst *CI, IRBuilder<> &B);
   Value *optimizeStrTo(CallInst *CI, IRBuilder<> &B);
   Value *optimizeStrSpn(CallInst *CI, IRBuilder<> &B);
   Value *optimizeStrCSpn(CallInst *CI, IRBuilder<> &B);

lib/Transforms/InstCombine/InstCombineCalls.cpp

 }
 
 /// Improvements for call, callbr and invoke instructions.
 Instruction *InstCombiner::visitCallBase(CallBase &Call) {
   if (isAllocationFn(&Call, &TLI))
     annotateAnyAllocSite(Call, &TLI);
 
   if (isAllocLikeFn(&Call, &TLI))
-    return visitAllocSite(Call);
+    if (Instruction *I = visitAllocSite(Call))
+      return I;
 
   bool Changed = false;
 
   // Mark any parameters that are known to be non-null with the nonnull
   // attribute.  This is helpful for inlining calls to functions with null
   // checks on their arguments.
   SmallVector<unsigned, 4> ArgNos;
   unsigned ArgNo = 0;

lib/Transforms/Utils/BuildLibCalls.cpp

 
 Value *llvm::emitStrLen(Value *Ptr, IRBuilder<> &B, const DataLayout &DL,
                         const TargetLibraryInfo *TLI) {
   LLVMContext &Context = B.GetInsertBlock()->getContext();
   return emitLibCall(LibFunc_strlen, DL.getIntPtrType(Context),
                      B.getInt8PtrTy(), castToCStr(Ptr, B), B, TLI);
 }
 
+Value *llvm::emitStrDup(Value *Ptr, IRBuilder<> &B,
+                        const TargetLibraryInfo *TLI) {
+  LLVMContext &Context = B.GetInsertBlock()->getContext();
+  return emitLibCall(LibFunc_strdup, B.getInt8PtrTy(), B.getInt8PtrTy(),
+                     castToCStr(Ptr, B), B, TLI);
+}
+
 Value *llvm::emitStrChr(Value *Ptr, char C, IRBuilder<> &B,
                         const TargetLibraryInfo *TLI) {
   Type *I8Ptr = B.getInt8PtrTy();
   Type *I32Ty = B.getInt32Ty();
   return emitLibCall(LibFunc_strchr, I8Ptr, {I8Ptr, I32Ty},
                      {castToCStr(Ptr, B), ConstantInt::get(I32Ty, C)}, B, TLI);
 }
 

lib/Transforms/Utils/SimplifyLibCalls.cpp

if (canTransformToMemCmp(CI, Str2P, Len1, DL))
           Str1P, Str2P,
           ConstantInt::get(DL.getIntPtrType(CI->getContext()), Len1), B, DL,
           TLI);
   }
 
   return nullptr;
 }
 
+Value *LibCallSimplifier::optimizeStrNDup(CallInst *CI, IRBuilder<> &B) {
+  Value *Src = CI->getArgOperand(0);
+  ConstantInt *Size = dyn_cast<ConstantInt>(CI->getArgOperand(1));
+  uint64_t SrcLen = GetStringLength(Src);
+  if (SrcLen && Size) {
+    annotateDereferenceableBytes(CI, 0, SrcLen);
+    if (!Size->isNullValue()) {
+      if (SrcLen <= Size->getZExtValue() + 1)
+        return emitStrDup(Src, B, TLI);
+    }
+  }
+
+  return nullptr;
+}
+
 Value *LibCallSimplifier::optimizeStrCpy(CallInst *CI, IRBuilder<> &B) {
   Value *Dst = CI->getArgOperand(0), *Src = CI->getArgOperand(1);
   if (Dst == Src) // strcpy(x,x)  -> x
     return Src;
   
   annotateNonNullBasedOnAccess(CI, {0, 1});
   // See if we can get the length of the input string.
   uint64_t Len = GetStringLength(Src);
if (TLI->getLibFunc(*Callee, Func) && TLI->has(Func)) {
     case LibFunc_stpcpy:
       return optimizeStpCpy(CI, Builder);
     case LibFunc_strncpy:
       return optimizeStrNCpy(CI, Builder);
     case LibFunc_strlen:
       return optimizeStrLen(CI, Builder);
     case LibFunc_strpbrk:
       return optimizeStrPBrk(CI, Builder);
+    case LibFunc_strndup:
+      return optimizeStrNDup(CI, Builder);
     case LibFunc_strtol:
     case LibFunc_strtod:
     case LibFunc_strtof:
     case LibFunc_strtoul:
     case LibFunc_strtoll:
     case LibFunc_strtold:
     case LibFunc_strtoull:
       return optimizeStrTo(CI, Builder);

test/Transforms/InstCombine/strndup.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -instcombine -S | FileCheck %s
+
+@hello = constant [6 x i8] c"hello\00"
+@null = constant [1 x i8] zeroinitializer
+
+declare i8* @strndup(i8*, i32)
+
+define i8* @test1() {
+; CHECK-LABEL: @test1(
+; CHECK-NEXT:    [[RET:%.*]] = call i8* @strndup(i8* dereferenceable(1) getelementptr inbounds ([1 x i8], [1 x i8]* @null, i64 0, i64 0), i32 0)
+; CHECK-NEXT:    ret i8* [[RET]]
+;
+  %src = getelementptr [1 x i8], [1 x i8]* @null, i32 0, i32 0
+  %ret = call i8* @strndup(i8* %src, i32 0)
+  ret i8* %ret
+}
+
+define i8* @test2() {
+; CHECK-LABEL: @test2(
+; CHECK-NEXT:    [[RET:%.*]] = call i8* @strndup(i8* dereferenceable(6) getelementptr inbounds ([6 x i8], [6 x i8]* @hello, i64 0, i64 0), i32 4)
+; CHECK-NEXT:    ret i8* [[RET]]
+;
+  %src = getelementptr [6 x i8], [6 x i8]* @hello, i32 0, i32 0
+  %ret = call i8* @strndup(i8* %src, i32 4)
+  ret i8* %ret
+}
+
+define i8* @test3() {
+; CHECK-LABEL: @test3(
+; CHECK-NEXT:    [[STRDUP:%.*]] = call dereferenceable_or_null(6) i8* @strdup(i8* getelementptr inbounds ([6 x i8], [6 x i8]* @hello, i64 0, i64 0))
+; CHECK-NEXT:    ret i8* [[STRDUP]]
+;
+  %src = getelementptr [6 x i8], [6 x i8]* @hello, i32 0, i32 0
+  %ret = call i8* @strndup(i8* %src, i32 5)
+  ret i8* %ret
+}
+
+define i8* @test4() {
+; CHECK-LABEL: @test4(
+; CHECK-NEXT:    [[STRDUP:%.*]] = call dereferenceable_or_null(6) i8* @strdup(i8* getelementptr inbounds ([6 x i8], [6 x i8]* @hello, i64 0, i64 0))
+; CHECK-NEXT:    ret i8* [[STRDUP]]
+;
+  %src = getelementptr [6 x i8], [6 x i8]* @hello, i32 0, i32 0
+  %ret = call i8* @strndup(i8* %src, i32 6)
+  ret i8* %ret
+}
+
+define i8* @test5() {
+; CHECK-LABEL: @test5(
+; CHECK-NEXT:    [[STRDUP:%.*]] = call dereferenceable_or_null(6) i8* @strdup(i8* getelementptr inbounds ([6 x i8], [6 x i8]* @hello, i64 0, i64 0))
+; CHECK-NEXT:    ret i8* [[STRDUP]]
+;
+  %src = getelementptr [6 x i8], [6 x i8]* @hello, i32 0, i32 0
+  %ret = call i8* @strndup(i8* %src, i32 7)
+  ret i8* %ret
+}
+
+define i8* @test6(i32 %n) {
+; CHECK-LABEL: @test6(
+; CHECK-NEXT:    [[RET:%.*]] = call i8* @strndup(i8* getelementptr inbounds ([6 x i8], [6 x i8]* @hello, i64 0, i64 0), i32 [[N:%.*]])
+; CHECK-NEXT:    ret i8* [[RET]]
+;
+  %src = getelementptr [6 x i8], [6 x i8]* @hello, i32 0, i32 0
+  %ret = call i8* @strndup(i8* %src, i32 %n)
+  ret i8* %ret
+}