[PATCH] D39607: [PartialInliner] Inline vararg functions that forward varargs.

[David Li via Phabricator via llvm-commits]

davidxl added a comment.

We do life-time marker and static alloca shrink wrapping optimization for outlining. Do you see opportunity for vaarg shrink-wrapping too?  If yes, it may be considered as a follow up.



================
Comment at: lib/Transforms/IPO/PartialInlining.cpp:153
+    // Do function outlining.
+    // NOTE: For vararg functions that do the vararg handling in the outlined
+    //       function, we temporarily generate IR that does not properly
----------------
This looks like stale comment.


================
Comment at: lib/Transforms/Utils/CodeExtractor.cpp:82
+bool CodeExtractor::isBlockValidForExtraction(const BasicBlock &BB,
+                                              bool AllowVarArgs) {
   // Landing pads must be in the function where they were inserted for cleanup.
----------------
Is there a need for this parameter? 


================
Comment at: lib/Transforms/Utils/CodeExtractor.cpp:119
       return false;
+    if (AllowVarArgs)
+      continue;
----------------
Having the check here may prevent other independent legality check (in the future) from taking effect. If it better to move it down.


================
Comment at: lib/Transforms/Utils/CodeExtractor.cpp:124
         if (F->getIntrinsicID() == Intrinsic::vastart)
           return false;
   }
----------------
if (AllowVarArgs)
   continue;
else
   return false;


================
Comment at: lib/Transforms/Utils/CodeExtractor.cpp:978
+        continue;
+      for (auto &I : BB)
+        if (const CallInst *CI = dyn_cast<CallInst>(&I))
----------------
nit: use std::any_of


https://reviews.llvm.org/D39607