[PATCH] D107872: [SimplifyLibCalls] propagate tail flag on FORTIFY calls

Nick Desaulniers via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 12 14:00:03 PDT 2021


nickdesaulniers added a comment.

In D107872#2940298 <https://reviews.llvm.org/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?



================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:3516
+    if (CI->isTailCall() && !SimplifiedCI->isTailCall())
+      SimplifiedCI->setTailCall();
+
----------------
efriedma wrote:
> 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.
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? 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107872/new/

https://reviews.llvm.org/D107872



More information about the llvm-commits mailing list