[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

4224–4225

sight, this breaks things (eg. zstd build)

[jdoerfert]

lib/Transforms/InstCombine/InstCombineCalls.cpp

4224–4225

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

4224–4225

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 {
   Value *castToCStr(Value *V, IRBuilder<> &B);
 
   /// 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
   /// 'intptr_t' type.
   Value *emitStrLen(Value *Ptr, IRBuilder<> &B, const DataLayout &DL,
                     const TargetLibraryInfo *TLI);
 
+  /// 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
+  /// '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

if (uint64_t Len = GetStringLength(Call.getOperand(0)))
           Attribute::getWithDereferenceableOrNullBytes(Call.getContext(), Len));
   }
 }
 
 /// 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);
-
   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;
 
   for (Value *V : Call.args()) {
Instruction *InstCombiner::visitCallBase(CallBase &Call) {
   // delete the instruction now.
   if (CallInst *CI = dyn_cast<CallInst>(&Call)) {
     Instruction *I = tryOptimizeCall(CI);
     // If we changed something return the result, etc. Otherwise let
     // the fallthrough check.
     if (I) return eraseInstFromFunction(*I);
   }
 
+  if (isAllocLikeFn(&Call, &TLI))
+    return visitAllocSite(Call);
+
   return Changed ? &Call : nullptr;
 }
 
 /// If the callee is a constexpr cast of a function, attempt to move the cast to
 /// the arguments of the call/callbr/invoke.
 bool InstCombiner::transformConstExprCastCall(CallBase &Call) {
   auto *Callee = dyn_cast<Function>(Call.getCalledValue()->stripPointerCasts());
   if (!Callee)

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) {
+  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 (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/objsize.ll

;
   %call = tail call i8* @strdup(i8* getelementptr inbounds ([8 x i8], [8 x i8]* @.str, i64 0, i64 0)) nounwind
   store i8* %call, i8** %esc, align 8
   %1 = tail call i32 @llvm.objectsize.i32.p0i8(i8* %call, i1 true, i1 false, i1 false)
   ret i32 %1
 }
 
 define i32 @test10(i8** %esc) {
 ; CHECK-LABEL: @test10(
-; CHECK-NEXT:    [[CALL:%.*]] = tail call i8* @strndup(i8* getelementptr inbounds ([8 x i8], [8 x i8]* @.str, i32 0, i32 0), i32 3) #0
+; CHECK-NEXT:    [[CALL:%.*]] = tail call i8* @strndup(i8* dereferenceable(8) getelementptr inbounds ([8 x i8], [8 x i8]* @.str, i32 0, i32 0), i32 3) #0
 ; CHECK-NEXT:    store i8* [[CALL]], i8** [[ESC:%.*]], align 8
 ; CHECK-NEXT:    ret i32 4
 ;
   %call = tail call i8* @strndup(i8* getelementptr inbounds ([8 x i8], [8 x i8]* @.str, i64 0, i64 0), i32 3) nounwind
   store i8* %call, i8** %esc, align 8
   %1 = tail call i32 @llvm.objectsize.i32.p0i8(i8* %call, i1 true, i1 false, i1 false)
   ret i32 %1
 }
 
 define i32 @test11(i8** %esc) {
 ; CHECK-LABEL: @test11(
-; CHECK-NEXT:    [[CALL:%.*]] = tail call i8* @strndup(i8* getelementptr inbounds ([8 x i8], [8 x i8]* @.str, i32 0, i32 0), i32 7) #0
-; CHECK-NEXT:    store i8* [[CALL]], i8** [[ESC:%.*]], align 8
+; CHECK-NEXT:    [[STRDUP:%.*]] = call dereferenceable_or_null(8) i8* @strdup(i8* getelementptr inbounds ([8 x i8], [8 x i8]* @.str, i32 0, i32 0))
+; CHECK-NEXT:    store i8* [[STRDUP]], i8** [[ESC:%.*]], align 8
 ; CHECK-NEXT:    ret i32 8
 ;
   %call = tail call i8* @strndup(i8* getelementptr inbounds ([8 x i8], [8 x i8]* @.str, i64 0, i64 0), i32 7) nounwind
   store i8* %call, i8** %esc, align 8
   %1 = tail call i32 @llvm.objectsize.i32.p0i8(i8* %call, i1 true, i1 false, i1 false)
   ret i32 %1
 }
 
 define i32 @test12(i8** %esc) {
 ; CHECK-LABEL: @test12(
-; CHECK-NEXT:    [[CALL:%.*]] = tail call i8* @strndup(i8* getelementptr inbounds ([8 x i8], [8 x i8]* @.str, i32 0, i32 0), i32 8) #0
-; CHECK-NEXT:    store i8* [[CALL]], i8** [[ESC:%.*]], align 8
+; CHECK-NEXT:    [[STRDUP:%.*]] = call dereferenceable_or_null(8) i8* @strdup(i8* getelementptr inbounds ([8 x i8], [8 x i8]* @.str, i32 0, i32 0))
+; CHECK-NEXT:    store i8* [[STRDUP]], i8** [[ESC:%.*]], align 8
 ; CHECK-NEXT:    ret i32 8
 ;
   %call = tail call i8* @strndup(i8* getelementptr inbounds ([8 x i8], [8 x i8]* @.str, i64 0, i64 0), i32 8) nounwind
   store i8* %call, i8** %esc, align 8
   %1 = tail call i32 @llvm.objectsize.i32.p0i8(i8* %call, i1 true, i1 false, i1 false)
   ret i32 %1
 }
 
 define i32 @test13(i8** %esc) {
 ; CHECK-LABEL: @test13(
-; CHECK-NEXT:    [[CALL:%.*]] = tail call i8* @strndup(i8* getelementptr inbounds ([8 x i8], [8 x i8]* @.str, i32 0, i32 0), i32 57) #0
-; CHECK-NEXT:    store i8* [[CALL]], i8** [[ESC:%.*]], align 8
+; CHECK-NEXT:    [[STRDUP:%.*]] = call dereferenceable_or_null(8) i8* @strdup(i8* getelementptr inbounds ([8 x i8], [8 x i8]* @.str, i32 0, i32 0))
+; CHECK-NEXT:    store i8* [[STRDUP]], i8** [[ESC:%.*]], align 8
 ; CHECK-NEXT:    ret i32 8
 ;
   %call = tail call i8* @strndup(i8* getelementptr inbounds ([8 x i8], [8 x i8]* @.str, i64 0, i64 0), i32 57) nounwind
   store i8* %call, i8** %esc, align 8
   %1 = tail call i32 @llvm.objectsize.i32.p0i8(i8* %call, i1 true, i1 false, i1 false)
   ret i32 %1
 }
 

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:    [[STRDUP:%.*]] = call dereferenceable_or_null(1) i8* @strdup(i8* getelementptr inbounds ([1 x i8], [1 x i8]* @null, i64 0, i64 0))
+; CHECK-NEXT:    ret i8* [[STRDUP]]
+;
+  %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
+}