[PATCH] D43729: [CallSiteSplitting] properly split musttail calls

[Jun Bum Lim via Phabricator via llvm-commits]

junbuml added a comment.

Thanks for catching this.

Different cloning using UnreachableInst could be a way to handle musttail. However, I prefer keeping the current way of splitting CS as it as. Instead, we may be able to handle musttail (clone bitcast/retrun and remove TailBB) at the end of splitCallSite() in a separate function.



================
Comment at: lib/Transforms/Scalar/CallSiteSplitting.cpp:268
+    // `DuplicateInstructionsInSplitBetween`.
+    StopAt = new UnreachableInst(TailBB->getContext(), TailBB);
+  } else {
----------------
Instead of cloning differently for musttail using UnreachableInst, I think it also possible to keep the current way of splitting as it is, but for musttail, we can handle it (clone bitcast and return and remove TailBB) as a separate function which being called at the end of this function. 


================
Comment at: lib/Transforms/Scalar/CallSiteSplitting.cpp:358
+  // Once this is done - remove the Tail itself.
+  if (IsMustTail) {
+    DEBUG(dbgs() << "clean up musttail splits\n");
----------------
Here, I think we can call a separate function which handle the musttail case. So we can specifically clone bitcast and return and remove TailBB.


================
Comment at: test/Transforms/CallSiteSplitting/musttail.ll:21
+  %ca = musttail call i8* @callee(i8* %a, i8* %b)
+  %cb = bitcast i8* %ca to i8*
+  ret i8* %cb
----------------
Can we add two more test cases : 
  1. return void .
  2. return without bitcast.


https://reviews.llvm.org/D43729