[PATCH] Teach the inliner how to preserve musttail invariants

Chandler Carruth chandlerc at gmail.com
Thu May 15 10:09:04 PDT 2014

Sorry for continually getting distracted here. I think this is essentially fine. Feel free to submit whenever, and we can figure out the comments and the confusing parts as we go. I don't think there is anything wrong here.

Comment at: lib/Transforms/Utils/InlineFunction.cpp:482-483
@@ -480,1 +481,4 @@
+/// Returns true if the given return instruction is immediately preceded by a
+/// musttail call or a bitcast immediately preceded by a musttail call.
+static CallInst *getPrecedingMustTailCall(ReturnInst *RI) {
Stale comment.

Comment at: lib/Transforms/Utils/InlineFunction.cpp:694-705
@@ -663,1 +693,14 @@
+  // We need to reduce the strength of any inlined tail calls.  For musttail, we
+  // have to avoid introducing potential unbounded stack growth.  For example,
+  // if functions 'f' and 'g' are mutually recursive with musttail, we can
+  // inline 'g' into 'f' so long as we preserve musttail on the cloned call to
+  // 'f'.  If either the inlined call site or the cloned call site is *not*
+  // musttail, the program already has one frame of stack growth, so it's safe
+  // to remove musttail.  Here is a table of example transformations:
+  //
+  //    f -> musttail g -> musttail f  ==>  f -> musttail f
+  //    f -> musttail g ->     tail f  ==>  f ->     tail f
+  //    f ->          g -> musttail f  ==>  f ->          f
+  //    f ->          g ->     tail f  ==>  f ->          f
+  bool InlinedMustTailCalls = false;
Would it make sense to sink this comment down to the TailCallKind bit below?

Comment at: lib/Transforms/Utils/InlineFunction.cpp:780-784
@@ -719,1 +779,7 @@
+    for (ReturnInst *RI : Returns) {
+      // Don't insert llvm.stackrestore calls after a musttail call, since the
+      // tail call will clear the whole stack frame.
+      if (InlinedMustTailCalls && getPrecedingMustTailCall(RI))
+        continue;
+      IRBuilder<>(RI).CreateCall(StackRestore, SavedPtr);
It's really surprising to me that this applies to *all* dynamic allocas rather than just to inalloca arguments... Maybe I'm just missing the point, but I'd appreciate a comment clarifying why this makes sense.

Is there a test case that covers this? I can't tell if some of the CHECK-NEXT ones actually cover this case.


More information about the llvm-commits mailing list