Don't Place Entry Safepoints Before the llvm.frameescape() Intrinsic

[swaroop.sridhar]

Summary

llvm.frameescape() intrinsic is not a real call. The intrinsic can only exist in the entry block.
Inserting a gc.statepoint() before llvm.frameescape() may split the entry block, and push the intrinsic out of the entry block.

[reames]

LGTM. Sorry for not getting to this review sooner. I managed not to see it.

Out of curiosity, why are you using frameescape with statepoints? Not a combination I would have expected.

[swaroop.sridhar]

The llvm.frameescape() intrinsic is inserted by the LLILC JIT to ensure that some information about generics-context stays alive.
The MSIL reader allocates memory on the stack to hold the some scratch information and marks it address-escaped.

[swaroop.sridhar]

Can you please checkin this change for me? Thanks.

llvm/trunk/lib/Transforms/Scalar/PlaceSafepoints.cpp

for (cursor = F.getEntryBlock().begin(); hasNextInstruction(cursor);
     // grow the stack (i.e. the call target has a non-zero frame
     // size).
     if (CallSite(cursor)) {
       if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(cursor)) {
         // llvm.assume(...) are not really calls.
         if (II->getIntrinsicID() == Intrinsic::assume) {
           continue;
         }
+        // llvm.frameescape() intrinsic is not a real call. The intrinsic can 
+        // exist only in the entry block.
+        // Inserting a statepoint before llvm.frameescape() may split the 
+        // entry block, and push the intrinsic out of the entry block.
+        if (II->getIntrinsicID() == Intrinsic::frameescape) {
+          continue;
+        }
       }
       break;
     }
   }
 
   assert((hasNextInstruction(cursor) || cursor->isTerminator()) &&
          "either we stopped because of a call, or because of terminator");
 

llvm/trunk/test/Transforms/PlaceSafepoints/statepoint-frameescape.ll

+; RUN: opt %s -S -place-safepoints | FileCheck %s
+
+declare void @llvm.frameescape(...)
+
+; Do we insert the entry safepoint after the frameescape intrinsic?
+define void @parent() gc "statepoint-example" {
+; CHECK-LABEL: @parent
+entry:
+; CHECK-LABEL: entry
+; CHECK-NEXT: alloca
+; CHECK-NEXT: frameescape
+; CHECK-NEXT: statepoint
+  %ptr = alloca i32
+  call void (...) @llvm.frameescape(i32* %ptr)
+  ret void
+}
+
+; This function is inlined when inserting a poll.  To avoid recursive 
+; issues, make sure we don't place safepoints in it.
+declare void @do_safepoint()
+define void @gc.safepoint_poll() {
+; CHECK-LABEL: gc.safepoint_poll
+; CHECK-LABEL: entry
+; CHECK-NEXT: do_safepoint
+; CHECK-NEXT: ret void 
+entry:
+  call void @do_safepoint()
+  ret void
+}