[SimplifyLibCalls] propagate tail flags on CallInsts

[nickdesaulniers]

Summary

I noticed we weren't propagating tail flags on calls when
FortifiedLibCallSimplifier.optimizeCall() was replacing calls to runtime
checked calls to the non-checked routines (when safe to do so). Make
sure to check this before replacing the original calls!

Also, avoid any libcall transforms when notail/musttail is present.

PR46734

[nickdesaulniers]

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

3041

There's another call site of FortifiedLibCallSimplifier.optimizeCall. Perhaps it would be better to sink this check into that method? Thoughts?

[nickdesaulniers]

• check in FortifiedLibCallSimplifier::optimizeCall

[nickdesaulniers]

FWIW, this was the test case I was looking at that triggered this: https://godbolt.org/z/864jvY5nr.

[xbolva00]

FYI https://reviews.llvm.org/D89249

This your patch looks more promising.

[nickdesaulniers]

In D107872#2939983, @xbolva00 wrote:

FYI https://reviews.llvm.org/D89249

This your patch looks more promising.

Ah! Also, I only do this for the FortifiedLibCallSimplifier::optimizeCall, maybe I should do this for the vanilla LibCallSimplifier as well?

[xbolva00]

So this fixes https://bugs.llvm.org/show_bug.cgi?id=46734

[efriedma]

On a sort-of-related note, should we avoid optimizing musttail and/or notail calls here?

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

3503

This isn't the right call in some cases, I think. For example, for memmove, this is the operand to the memmove, not the memmove itself.

[nickdesaulniers]

In D107872#2940298, @efriedma wrote:

On a sort-of-related note, should we avoid optimizing musttail and/or notail calls here?

RE: notail:
Not necessary. If we have a notail call, we transform it, but we don't add tail so nothing to see here.

RE: musttail:
Probably; if I replace a musttail with a non-musttail call, that's kind of the same problem I'm trying to fix here with tail. Though when I try that, I get an error I don't yet understand:

cannot guarantee tail call due to mismatched parameter counts
  %ret = musttail call i32 @__vsprintf_chk(i8* %dst, i32 0, i64 -1, i8* %src, %struct.__va_list_tag* null)

huh?

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

3503

Yeah, I don't quite understand not returning the newly constructed call instruction. Does that mean that the call to replaceAllUsesWith on line 3041 wont actually insert the new call instruction, or rather replace the old instruction?

[nickdesaulniers]

So this fixes https://bugs.llvm.org/show_bug.cgi?id=46734

It does not (yet, but could+should), probably because of the case that @efriedma describes above. I may want to do:

if (CI->isTailCall())
      SimplifiedCI->setTailCall();

within each optimize* method.

[xbolva00]

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

3503

memmove/fortified memmove returns a pointer, llvm.memmove and friends return “void”

[efriedma]

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

3503

The code at line 3041 is probably not working as intended, yes.

Actually, I'm not sure what that code is trying to do; not sure why we don't want to just let the instcombine worklist do its normal thing.

[nickdesaulniers]

• add lots of tests, and fixes for those tests

[nickdesaulniers]

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

3503

Ah, looks like there's seperate test files for a few of these.

• memcpy_chk-1.ll, memcpy_chk-2.ll
• memset_chk-1.ll, memset_chk-2.ll
• stpcpy_chk-1.ll
• strcpy_chk-64.ll, strcpy_chk-2.ll

Also, in https://bugs.llvm.org/show_bug.cgi?id=46734, it looks like whatever converts calls to __memcpy_chk to @llvm.memcpy.p0i8.p0i8.i64 is also dropping these!!!

Oh god, it's endemic to the non-chk/vanilla LibCallSimplifier, too! Should I litter the code with these, or should we consider running TailCallElim immediately after LibcallSimplifier?

[nickdesaulniers]

or should we consider running TailCallElim immediately after LibcallSimplifier?

Sorry, I guess that would be s/LibcallSimplifier/instcombine.

[nickdesaulniers]

TODO: of the three cases reported in https://bugs.llvm.org/show_bug.cgi?id=46734, this still missed the memmove case.

[xbolva00]

or should we consider running TailCallElim immediately after LibcallSimplifier?

Yeah, would be great ro insert it after loop idiom pass.

[nickdesaulniers]

• memmove, mempcpy

[nickdesaulniers]

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

3267–3271

if this pattern of:

1. set attributes from new to old
2. remove type incompatible attributes
3. copy over tail flag (maybe musttail)

Then perhaps this should be a helper function; copyAttributesFlags(const CallInst *From, CallInst *To)? Though I'd like @efriedma 's input on this approach of redoing work another pass does, vs inserting another run of TailCallElim after instcombine?

[nickdesaulniers]

In D107872#2940298, @efriedma wrote:

On a sort-of-related note, should we avoid optimizing musttail and/or notail calls here?

Yes, looks like

New->setTailCallKind(Old.getTailCallKind()); would be better.

[nickdesaulniers]

• rebase, rewrite approach w/helper, add flag propagation everywhere

[xbolva00]

Nice approach, +1.

[nickdesaulniers]

@xbolva00 you've made 28 of the last 100 commits to llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp; I think that means you own it now. Is that +1 a formal accept of the patch or are we waiting on additional review?

[xbolva00]

If possible, @efriedma should take a look as well.

[jdoerfert]

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

610

Mostly this makes sense but doesn't this violate tail properties now? The old call result was return but it is not anymore.

[nickdesaulniers]

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

610

perhaps we should be avoiding libcall transforms that don't return the new CallInst if the CallInst to be replaced is marked musttail? Let me add musttail tests.

[xbolva00]

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

610

You mean eg musttail on strncpy?

Well yeah, but anyway there are many other libcall simplifications which break “musttail” contract (so if CI is musttail, we should not perform some simplifications).

But yeah, now with propagated tail kind, some IR verifier or backend may fail.

Nick, can you try it please?

[nickdesaulniers]

• avoid transforms when musttail and not replacing a call with a new call, add 11 new musttail tests

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

610

Yeah, there's 11 cases that I'm updating from Diff 377048. I will write 11 new tests, too.

[jdoerfert]

LG, with some missing checks that can be added for the final version.

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

1127

Musttail check for this one?

1342

Also not allowed to be musttail.

1599

copy flags only if not musstail or we actually return "this Sqrt".

2574

Musttail check?

2610

Musttail check?

3318

make it return copyFlags so it's easy to see this doesn't need a guard.

[xbolva00]

If we replace musttail CI with non-CI or CI without musttail, maybe we should have an assert somewhere to detect this issue for new libcall simplifications

[nickdesaulniers]

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

1127

oof, and the case above it, too.

1170

missing musttail

[nickdesaulniers]

• add more musttail guards, a musttail assert, 18 new tests

[nickdesaulniers]

In D107872#3059254, @xbolva00 wrote:

If we replace musttail CI with non-CI or CI without musttail, maybe we should have an assert somewhere to detect this issue for new libcall simplifications

Done. PTAL @ asserts added to InstCombinerImpl::tryOptimizeCall in llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp.

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

1599

This case is tricky. Please review my changes here and to llvm/test/Transforms/InstCombine/pow-1.ll carefully. I think I have it right though, since the below transforms replace the call to pow with non-CallInsts, while above are CallInsts.

[nickdesaulniers]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2435–2444 ↗

(On Diff #379205)

Thinking about this overnight...if this is the one place where replacement occurs, then why don't I just copy over the tail call kind here, rather than use the adapter in multiple locations in LibCallSimplifier? As I've already demonstrating, this approach is error prone; perhaps I could "do the work" here. Then we wouldn't need the asserts here, and this becomes a smaller change. Let me give that a shot and see if there's something I'm missing here as to why that can't be done.

[xbolva00]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2435–2444 ↗

(On Diff #379205)

This is what was the idea of https://reviews.llvm.org/D89249 but check @efriedma 's comment. First point is problematic.

For second and third:

"A lot of combines create calls, but return some other value."

I think this is not a issue. We simply fail to preserve this info, but what is problematic .. musttail. I still think if CI->isMustTailCall() == true, than we should throw away NewCI if NewCI is not a CallInst.

"For the floating-point routines, we probably want to add "tail" unconditionally."

Yeah, probably okay and can be here. Not sure how to correctly detect it, maybe TLI.has(rountine) && rountine->getType() is FP?

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

1130

Is it worth it at all?

I would just bail out libcall simplifier for musttail CIs

[nickdesaulniers]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2435–2444 ↗

(On Diff #379205)

I think this is not a issue. We simply fail to preserve this info, but what is problematic .. musttail. I still think if CI->isMustTailCall() == true, than we should throw away NewCI if NewCI is not a CallInst.

I agree. Let me comment on D89249; after spending too much time looking into this, I think that is the right approach. With some modifications to D89249, I think we should land that, then I'll rebase all these tests I've added onto that. Sound good?

[xbolva00]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2435–2444 ↗

(On Diff #379205)

Yeah,, this approach sounds better.

[efriedma]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2440 ↗

(On Diff #379205)

I don't understand this assertion. There are two possibilities for simplification:

1. We replace a call with some existing value.
2. We replace a call with another new call.

For (1), this assertion doesn't make sense: we don't have a call. For (2), we probably can't safely do the replacement in general; musttail has a bunch of restrictions, and the replacement call might not satisfy them.

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

1130

I agree; musttail calls should be rare enough that it isn't a big deal to just skip libcall simplification completely

[nickdesaulniers]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2440 ↗

(On Diff #379205)

These asserts were added in response to https://reviews.llvm.org/D107872#3059254. I think they do make sense, from the perspective of guarding against musttail calls being transformed.

If the old call is musttail, and the recommended replacement is not also musttail, assert that the suggestion is probably broken.

It is anomalous if a must tail call was simplified to a value. (1)

It is anomalous if a must tail call was simplified to a new call that's also not must tail.

Though if we just avoid the call to Simplifier.optimizeCall above if CI is must tail, then these asserts become impossible and can be removed.

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

1130

So just completely skipping the call to LibCallSimplifier::optimizeCall from InstCombinerImpl::tryOptimizeCall if the call is must tail? That's going to require me to rewrite a lot of the newly added test cases here; please confirm we're in mutual understanding before I go and make such invasive changes to this patch.

[efriedma]

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

1130

Yes, this.

[nickdesaulniers]

• skip musttail calls and update musttail tests, add asserts, fix a few floating point cases

[nickdesaulniers]

Ok, rather than a bunch of checks for musttail, I've centralized it to one place (ie. skipping libcall simplifications for musttail). I keep the manual checks, and add an assert to hopefull catch any new transforms that accidentally drop the tail and notail flags.

[nickdesaulniers]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2427–2429 ↗

(On Diff #382147)

There's probably a better way to express/word this. @efriedma please let me know what to replace here.

[nickdesaulniers]

bumping for review (@efriedma )

[efriedma]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2427–2429 ↗

(On Diff #382147)

I'd probably say "Skip optimizing musttail calls so LibCallSimplifier::optimizeCall doesn't have to preserve those invariants."

Actually, to be on the safe side, probably best to also skip optimizing notail calls... I'm not what "notail" actually means on a libcall, if anything, but might as well be conservative.

2440 ↗

(On Diff #379205)

I still think the assertion doesn't make sense.

In the "We replace a call with some existing value" case, consider, for example, memcpy. We have an optimization that transforms memcpy->llvm.memcpy. llvm.memcpy, doesn't have a return value, though, so we emit an llvm.memcpy, and return the value of the first operand. This operand may or may not be a call, and that call may or may not be a tail call.

[nickdesaulniers]

• rebase, avoid notail, update comment and description

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2440 ↗

(On Diff #379205)

For the case you describe, With will not be the CallInst to llvm.memcpy; it will be the destination operand of the original call to memcpy, so the dyn_cast will return nullptr. Thus the assertion is not checked at runtime for the case you describe.

The assertion is only checked when we replace a CallInst with another CallInst (with the new CallInst being what's returned from the call to LibCallSimplifier::optimizeCall).

To me, the assertion is a guard for future changes to LibCallSimplifier to ensure copyFlags is done safely in a manner that is done throughout this diff.

[nickdesaulniers]

bumping for review (@efriedma ).

[nickdesaulniers]

bumping for review (@efriedma ).

[efriedma]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2440 ↗

(On Diff #379205)

it will be the destination operand of the original call to memcpy

Yes.

so the dyn_cast will return nullptr

I'm not following; is there some reason the destination can't be an instruction?

[nickdesaulniers]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2440 ↗

(On Diff #379205)

so the dyn_cast will return nullptr

I'm not following; is there some reason the destination can't be an instruction?

It might be an Instruction, but not necessarily a CallInst. The dyn_cast is testing whether the returned Value is a CallInst and only checking that the tail call kind flag was propagate in that case. In all other cases of different Values returned, the assertion is not checked.

---

Or are you saying that the first operand to a call to memcpy could itself be a CallInst? Is that valid/possible to do in IR? ie.

%bar = call i8* memcpy(call i8* @foo(), i8* %src, i64 %n)

This operand may or may not be a call, and that call may or may not be a tail call.

I guess I'm curious how can the first operand be a call? Perhaps:

%dest = call i8* @foo()
%bar = call i8* @memcpy(i8* %dest, i8* %src, i64 %n)

Does that mean that %dest is a CallInst? If yes, then I indeed should remove the assert, but can you please confirm for me?

[efriedma]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2440 ↗

(On Diff #379205)

In the following, %dest is a CallInst, yes.

%dest = call i8* @foo()
%bar = call i8* @memcpy(i8* %dest, i8* %src, i64 %n)

[nickdesaulniers]

• add test case of CallInst first param to memcpy and remove assertion, as per @efriedma

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2440 ↗

(On Diff #379205)

Ok, let me add that as a test case, and drop the assertion. PTAL

[nickdesaulniers]

bumping for review (@efriedma ).

[nickdesaulniers]

bumping for review (@efriedma or anyone else; I plan to land this next Monday if there's no further feedback).

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

Value *LibCallSimplifier::optimizeStrNCpy(CallInst *CI, IRBuilderBase &B) {
   Type *PT = Callee->getFunctionType()->getParamType(0);
   // strncpy(x, s, c) -> memcpy(align 1 x, align 1 s, c) [s and c are constant]
   CallInst *NewCI = B.CreateMemCpy(Dst, Align(1), Src, Align(1),
                                    ConstantInt::get(DL.getIntPtrType(PT), Len));
   NewCI->setAttributes(CI->getAttributes());
   NewCI->removeAttributes(AttributeList::ReturnIndex,
                           AttributeFuncs::typeIncompatible(NewCI->getType()));
   return Dst;
 }
 
 Value *LibCallSimplifier::optimizeStringLength(CallInst *CI, IRBuilderBase &B,
                                                unsigned CharSize) {
   Value *Src = CI->getArgOperand(0);
 
   // Constant folding: strlen("xyz") -> 3
   if (uint64_t Len = GetStringLength(Src, CharSize))
     return ConstantInt::get(CI->getType(), Len - 1);
Value *LibCallSimplifier::optimizeMemCCpy(CallInst *CI, IRBuilderBase &B) {
 
   Value *NewN =
       ConstantInt::get(N->getType(), std::min(uint64_t(Pos + 1), N->getZExtValue()));
   // memccpy -> llvm.memcpy
   B.CreateMemCpy(Dst, Align(1), Src, Align(1), NewN);
   return Pos + 1 <= N->getZExtValue()
              ? B.CreateInBoundsGEP(B.getInt8Ty(), Dst, NewN)
              : Constant::getNullValue(CI->getType());
 }
 
 Value *LibCallSimplifier::optimizeMemPCpy(CallInst *CI, IRBuilderBase &B) {
   Value *Dst = CI->getArgOperand(0);
   Value *N = CI->getArgOperand(2);
   // mempcpy(x, y, n) -> llvm.memcpy(align 1 x, align 1 y, n), x + n
   CallInst *NewCI =
       B.CreateMemCpy(Dst, Align(1), CI->getArgOperand(1), Align(1), N);
   // Propagate attributes, but memcpy has no return value, so make sure that
   // any return attributes are compliant.
   // TODO: Attach return value attributes to the 1st operand to preserve them?
   NewCI->setAttributes(CI->getAttributes());
Value *LibCallSimplifier::optimizeMemSet(CallInst *CI, IRBuilderBase &B) {
   if (isa<IntrinsicInst>(CI))
     return nullptr;
 
   // 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);
 }
 
 Value *LibCallSimplifier::optimizeRealloc(CallInst *CI, IRBuilderBase &B) {
   if (isa<ConstantPointerNull>(CI->getArgOperand(0)))
     return emitMalloc(CI->getArgOperand(1), B, DL, TLI);
 
   return nullptr;
static Value *optimizeTrigReflections(CallInst *Call, LibFunc Func,
   case LibFunc_sinl:
   case LibFunc_tan:
   case LibFunc_tanf:
   case LibFunc_tanl:
     // sin(-X) --> -sin(X)
     // tan(-X) --> -tan(X)
     if (match(Call->getArgOperand(0), m_OneUse(m_FNeg(m_Value(X)))))
       return B.CreateFNeg(B.CreateCall(Call->getCalledFunction(), X));
     break;
   case LibFunc_cos:
   case LibFunc_cosf:
   case LibFunc_cosl:
     // cos(-X) --> cos(X)
     if (match(Call->getArgOperand(0), m_FNeg(m_Value(X))))
       return B.CreateCall(Call->getCalledFunction(), X, "cos");
     break;
   default:
Value *LibCallSimplifier::replacePowWithSqrt(CallInst *Pow, IRBuilderBase &B) {
 
   Sqrt = getSqrtCall(Base, Attrs, Pow->doesNotAccessMemory(), Mod, B, TLI);
   if (!Sqrt)
     return nullptr;
 
   // Handle signed zero base by expanding to fabs(sqrt(x)).
   if (!Pow->hasNoSignedZeros()) {
     Function *FAbsFn = Intrinsic::getDeclaration(Mod, Intrinsic::fabs, Ty);
     Sqrt = B.CreateCall(FAbsFn, Sqrt, "abs");
   }
 
   // Handle non finite base by expanding to
   // (x == -infinity ? +infinity : sqrt(x)).
   if (!Pow->hasNoInfs()) {
     Value *PosInf = ConstantFP::getInfinity(Ty),
           *NegInf = ConstantFP::getInfinity(Ty, true);
     Value *FCmp = B.CreateFCmpOEQ(Base, NegInf, "isinf");
if (CI->getNumArgOperands() == 3) {
     B.CreateMemCpy(
         CI->getArgOperand(0), Align(1), CI->getArgOperand(2), Align(1),
         ConstantInt::get(DL.getIntPtrType(CI->getContext()),
                          FormatStr.size() + 1)); // Copy the null byte.
     return ConstantInt::get(CI->getType(), FormatStr.size());
   }
 
   // The remaining optimizations require the format string to be "%s" or "%c"
   // and have an extra operand.
   if (FormatStr.size() == 2 && FormatStr[0] == '%' &&
       CI->getNumArgOperands() == 4) {
 
     // Decode the second character of the format string.
     if (FormatStr[1] == 'c') {
       if (N == 0)
         return ConstantInt::get(CI->getType(), 1);
       else if (N == 1)
if (FormatStr[1] == 's') {
 
       if (N == 0)
         return ConstantInt::get(CI->getType(), Str.size());
       else if (N < Str.size() + 1)
         return nullptr;
 
       B.CreateMemCpy(CI->getArgOperand(0), Align(1), CI->getArgOperand(3),
                      Align(1), ConstantInt::get(CI->getType(), Str.size() + 1));
 
       // The snprintf result is the unincremented number of bytes in the string.
       return ConstantInt::get(CI->getType(), Str.size());
     }
   }
   return nullptr;
 }
 
 Value *LibCallSimplifier::optimizeSnPrintF(CallInst *CI, IRBuilderBase &B) {
Value *LibCallSimplifier::optimizeCall(CallInst *CI, IRBuilderBase &Builder) {
   }
 
   // Also try to simplify calls to fortified library functions.
   if (Value *SimplifiedFortifiedCI =
           FortifiedSimplifier.optimizeCall(CI, Builder)) {
     // Try to further simplify the result.
     CallInst *SimplifiedCI = dyn_cast<CallInst>(SimplifiedFortifiedCI);
     if (SimplifiedCI && SimplifiedCI->getCalledFunction()) {
+
+      if (CI->isTailCall() && !SimplifiedCI->isTailCall())
+        SimplifiedCI->setTailCall();
+
       // Ensure that SimplifiedCI's uses are complete, since some calls have
       // their uses analyzed.
       replaceAllUsesWith(CI, SimplifiedCI);
 
       // Set insertion point to SimplifiedCI to guarantee we reach all uses
       // we might replace later on.
       IRBuilderBase::InsertPointGuard Guard(Builder);
       Builder.SetInsertPoint(SimplifiedCI);

 
 Value *FortifiedLibCallSimplifier::optimizeMemPCpyChk(CallInst *CI,
                                                       IRBuilderBase &B) {
   const DataLayout &DL = CI->getModule()->getDataLayout();
   if (isFortifiedCallFoldable(CI, 3, 2))
     if (Value *Call = emitMemPCpy(CI->getArgOperand(0), CI->getArgOperand(1),
                                   CI->getArgOperand(2), B, DL, TLI)) {
       CallInst *NewCI = cast<CallInst>(Call);
       NewCI->setAttributes(CI->getAttributes());
       NewCI->removeAttributes(
           AttributeList::ReturnIndex,
           AttributeFuncs::typeIncompatible(NewCI->getType()));
       return NewCI;
     }
   return nullptr;
 }
 
 Value *FortifiedLibCallSimplifier::optimizeStrpCpyChk(CallInst *CI,
                                                       IRBuilderBase &B,
                                                       LibFunc Func) {
   const DataLayout &DL = CI->getModule()->getDataLayout();
Value *FortifiedLibCallSimplifier::optimizeStrpCpyChk(CallInst *CI,
 
   Type *SizeTTy = DL.getIntPtrType(CI->getContext());
   Value *LenV = ConstantInt::get(SizeTTy, Len);
   Value *Ret = emitMemCpyChk(Dst, Src, LenV, ObjSize, B, DL, TLI);
   // If the function was an __stpcpy_chk, and we were able to fold it into
   // a __memcpy_chk, we still need to return the correct end pointer.
   if (Ret && Func == LibFunc_stpcpy_chk)
     return B.CreateGEP(B.getInt8Ty(), Dst, ConstantInt::get(SizeTTy, Len - 1));
   return Ret;
 }
 
 Value *FortifiedLibCallSimplifier::optimizeStrLenChk(CallInst *CI,
                                                      IRBuilderBase &B) {
   if (isFortifiedCallFoldable(CI, 1, None, 0))
     return emitStrLen(CI->getArgOperand(0), B, CI->getModule()->getDataLayout(),
                       TLI);
   return nullptr;
Value *FortifiedLibCallSimplifier::optimizeCall(CallInst *CI,
   case LibFunc_vsprintf_chk:
     return optimizeVSPrintfChk(CI, Builder);
   default:
     break;
   }
   return nullptr;
 }
 
 FortifiedLibCallSimplifier::FortifiedLibCallSimplifier(
     const TargetLibraryInfo *TLI, bool OnlyLowerUnknownSize)
     : TLI(TLI), OnlyLowerUnknownSize(OnlyLowerUnknownSize) {}

llvm/test/Transforms/InstCombine/fortify-folding.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -instcombine -S | FileCheck %s
 
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 
 @a = common global [60 x i8] zeroinitializer, align 1
 @b = common global [60 x i8] zeroinitializer, align 1
 @.str = private constant [12 x i8] c"abcdefghijk\00"
 
 %struct.__va_list_tag = type { i32, i32, i8*, i8* }
 
 define i8* @test_memccpy() {
 ; CHECK-LABEL: @test_memccpy(
-; CHECK-NEXT:    [[MEMCCPY:%.*]] = call i8* @memccpy(i8* getelementptr inbounds ([60 x i8], [60 x i8]* @a, i64 0, i64 0), i8* getelementptr inbounds ([60 x i8], [60 x i8]* @b, i64 0, i64 0), i32 0, i64 60)
+; CHECK-NEXT:    [[MEMCCPY:%.*]] = tail call i8* @memccpy(i8* getelementptr inbounds ([60 x i8], [60 x i8]* @a, i64 0, i64 0), i8* getelementptr inbounds ([60 x i8], [60 x i8]* @b, i64 0, i64 0), i32 0, i64 60)
 ; CHECK-NEXT:    ret i8* [[MEMCCPY]]
 ;
   %dst = getelementptr inbounds [60 x i8], [60 x i8]* @a, i32 0, i32 0
   %src = getelementptr inbounds [60 x i8], [60 x i8]* @b, i32 0, i32 0
-  %ret = call i8* @__memccpy_chk(i8* %dst, i8* %src, i32 0, i64 60, i64 -1)
+  %ret = tail call i8* @__memccpy_chk(i8* %dst, i8* %src, i32 0, i64 60, i64 -1)
   ret i8* %ret
 }
 
 define i8* @test_not_memccpy() {
 ; CHECK-LABEL: @test_not_memccpy(
 ; CHECK-NEXT:    [[RET:%.*]] = call i8* @__memccpy_chk(i8* getelementptr inbounds ([60 x i8], [60 x i8]* @a, i64 0, i64 0), i8* getelementptr inbounds ([60 x i8], [60 x i8]* @b, i64 0, i64 0), i32 0, i64 60, i64 59)
 ; CHECK-NEXT:    ret i8* [[RET]]
 ;