[PATCH] Teach the inliner how to preserve musttail invariants

[Chandler Carruth]

================
Comment at: lib/IR/Instructions.cpp:327
@@ -326,3 +326,3 @@
   setAttributes(CI.getAttributes());
-  setTailCall(CI.isTailCall());
+  setTailCallKind(CI.getTailCallKind());
   setCallingConv(CI.getCallingConv());
----------------
Please commit this separately with a separate test? Maybe a unittest?

================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:483
@@ -480,1 +482,3 @@
 
+static bool isPrecededByMustTailCall(const Instruction *Inst) {
+  const Instruction *Prev = Inst->getPrevNode();
----------------
This is called in a context that needs it to not go truly linear... And it doesn't. I almost want to call it 'isImmediatelyPrecededByMustTailCall', but the bitcast thing makes that a lie. Maybe just some comments?

================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:687-690
@@ +686,6 @@
+  //
+  //    f -> musttail g -> musttail h  ==>  f -> musttail g
+  //    f -> musttail g ->     tail h  ==>  f ->     tail g
+  //    f ->          g -> musttail h  ==>  f ->          g
+  //    f ->          g ->     tail h  ==>  f ->          g
+  //
----------------
I assume the after cases should have "h"? Actually, shouldn't these be "f -> g -> f" => "f -> f"?

================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:692-693
@@ +691,4 @@
+  //
+  // Also, calls inlined through a 'nounwind' call site should be marked
+  // 'nounwind'.
+  bool InlinedMustTailCalls = false;
----------------
I would sink this comment to the loop body.

================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:744-745
@@ +743,4 @@
+      for (ReturnInst *RI : Returns) {
+        // Don't insert lifetime.end calls after a musttail call, the allocas
+        // are trivially dead.
+        if (InlinedMustTailCalls && isPrecededByMustTailCall(RI))
----------------
Do all of the passes using lifetime markers know this?

It might be useful to separate this into its own patch with its own test case.

================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:795
@@ +794,3 @@
+      // Change users of the return value of the call.  For a musttail call
+      // site, the only real user should be a return instruction with an
+      // optional bitcast.  Avoid chains of bitcasts.
----------------
You say "should" but then "assert" below. I think the should is correct though...

================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:804
@@ +803,3 @@
+          RV = OldCast->getOperand(0);
+        assert(cast<CallInst>(RV)->isMustTailCall());
+
----------------
So, I don't think you can *always* assert. What about a musttail call to a function that just happens to return the result of a non-musttail call? Above, you set the flag if any call is musttail, so it seems you should just continue if this call site isn't a musttail.

Similarly, I don't think you can assert that there will be a callinst here.

http://reviews.llvm.org/D3491