[PATCH] D22529: Inlining of empty/small variadic functions.

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 28 23:09:11 PDT 2016


chandlerc added a comment.

In https://reviews.llvm.org/D22529#556295, @Sunil_Srivastava wrote:

> This is an update to inlining-of-variadic-functions change along the lines of suggestion by Chandler.
>
> The determination to attempt or not is being made in InlineCost.cpp.
>
> Presence of a va_start or a MustTail call in a variadic callee disqualifies it from being inlined. Other than that they can be inlined.


What's the motivation to inline cases where a variadic function calls some other variadic function but not with 'musttail'? I understand that such a call can't usefully call 'va_start', but such a call seems unlikely to matter regardless? If this is important to handle for some reason, I think it would be important to understand why. If it isn't important, it would be much simpler to just match the old behavior but inside inline cost where we can ignore uses along dead code paths.

I am a bit worried about C code with direct, trivial forwarding from one function to another without anything *explicitly* marking the inner call such that it is musttail, and just relying on a tail call occurring....

Some minor nits below, not sure they will be relevant depending on the answer to the issues above.


================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:1751
@@ -1748,2 +1750,3 @@
 
+    bool calleeIsVararg = CalledFunc->isVarArg(); // Just for asserts below.
     for (Function::iterator BB = FirstNewBlock, E = Caller->end(); BB != E;
----------------
nit: wrong naming convention for variable...

================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:1758-1761
@@ -1754,2 +1757,6 @@
           continue;
+	if (calleeIsVararg) {
+          assert((!CI->isMustTailCall())
+                        && "Inlined musttailcall in vararg function");
+	}
 
----------------
nit: formatting is weird here.


https://reviews.llvm.org/D22529





More information about the llvm-commits mailing list