[PATCH] D92657: Fix interaction between clang and some inline builtins from glibc under _FORTIFY_SOURCE

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added inline comments.


================
Comment at: llvm/lib/Analysis/InlineCost.cpp:2452-2455
       Function *Callee = Call->getCalledFunction();
+      // Disallow recursive calls.
       if (&F == Callee)
         return InlineResult::failure("recursive call");
----------------
shouldn't we do something here with `RecursiveIsViable`?


================
Comment at: llvm/lib/Analysis/InlineCost.cpp:2494
+InlineResult llvm::isAlwaysInlineViable(Function &F) {
+  return isInlineViableImpl(F, false /* allow recursive call */);
+}
----------------
should the second parameter here be `true`? If so, implies missing test coverage.


================
Comment at: llvm/lib/Analysis/InlineCost.cpp:2495
+  return isInlineViableImpl(F, false /* allow recursive call */);
+}
+
----------------
rather than splitting this up and adding parameters, I think we can just check for the alwaysinline fn attr in `llvm::isInlineViable`.


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

https://reviews.llvm.org/D92657