[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