[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.

http://reviews.llvm.org/D3491






More information about the llvm-commits mailing list