From a5703bc52e0196e8ef560a488d8fab500dfb8507 Mon Sep 17 00:00:00 2001
From: Philip Reames <listmail@philipreames.com>
Date: Tue, 19 May 2015 23:40:11 +0000
Subject: [PATCH] [PlaceSafepoints] Stop special casing some intrinsics

We were special casing a handful of intrinsics as not needing a safepoint before them.  After running into another valid case - memset - I took a closer look and realized that almost no intrinsics need to have a safepoint poll before them.  Restructure the code to make that apparent so that we stop hitting these bugs.  The only intrinsics which need a safepoint poll before them are ones which can run arbitrary code.



git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@237744 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/Transforms/Scalar/PlaceSafepoints.cpp | 53 +++++++++++++++--------
 test/Transforms/PlaceSafepoints/memset.ll | 20 +++++++++
 2 files changed, 56 insertions(+), 17 deletions(-)
 create mode 100644 test/Transforms/PlaceSafepoints/memset.ll

diff --git a/lib/Transforms/Scalar/PlaceSafepoints.cpp b/lib/Transforms/Scalar/PlaceSafepoints.cpp
index a6356da2ecb..fc1453b3175 100644
--- a/lib/Transforms/Scalar/PlaceSafepoints.cpp
+++ b/lib/Transforms/Scalar/PlaceSafepoints.cpp
@@ -381,6 +381,32 @@ bool PlaceBackedgeSafepointsImpl::runOnLoop(Loop *L) {
   return false;
 }
 
+/// Returns true if an entry safepoint is not required before this callsite in
+/// the caller function.  
+static bool doesNotRequireEntrySafepointBefore(const CallSite &CS) {
+  Instruction *Inst = CS.getInstruction();
+  if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(Inst)) {
+    switch (II->getIntrinsicID()) {
+    case Intrinsic::experimental_gc_statepoint:
+    case Intrinsic::experimental_patchpoint_void:
+    case Intrinsic::experimental_patchpoint_i64:
+      // The can wrap an actual call which may grow the stack by an unbounded
+      // amount or run forever.
+      return false;
+    default:
+      // Most LLVM intrinsics are things which do not expand to actual calls, or
+      // at least if they do, are leaf functions that cause only finite stack
+      // growth.  In particular, the optimizer likes to form things like memsets
+      // out of stores in the original IR.  Another important example is
+      // llvm.frameescape which must occur in the entry block.  Inserting a
+      // safepoint before it is not legal since it could push the frameescape
+      // out of the entry block.
+      return true;
+    }
+  }
+  return false;
+}
+
 static Instruction *findLocationForEntrySafepoint(Function &F,
                                                   DominatorTree &DT) {
 
@@ -421,23 +447,16 @@ static Instruction *findLocationForEntrySafepoint(Function &F,
   for (cursor = F.getEntryBlock().begin(); hasNextInstruction(cursor);
        cursor = nextInstruction(cursor)) {
 
-    // We need to stop going forward as soon as we see a call that can
-    // 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;
-        }
-      }
+    // We need to ensure a safepoint poll occurs before any 'real' call.  The
+    // easiest way to ensure finite execution between safepoints in the face of
+    // recursive and mutually recursive functions is to enforce that each take
+    // a safepoint.  Additionally, we need to ensure a poll before any call
+    // which can grow the stack by an unbounded amount.  This isn't required
+    // for GC semantics per se, but is a common requirement for languages
+    // which detect stack overflow via guard pages and then throw exceptions.
+    if (auto CS = CallSite(cursor)) {
+      if (doesNotRequireEntrySafepointBefore(CS))
+        continue;
       break;
     }
   }
diff --git a/test/Transforms/PlaceSafepoints/memset.ll b/test/Transforms/PlaceSafepoints/memset.ll
new file mode 100644
index 00000000000..534b2f12058
--- /dev/null
+++ b/test/Transforms/PlaceSafepoints/memset.ll
@@ -0,0 +1,20 @@
+; RUN: opt -S -place-safepoints %s | FileCheck %s
+
+define void @test(i32, i8 addrspace(1)* %ptr) gc "statepoint-example" {
+; CHECK-LABEL: @test
+; CHECK-NEXT: llvm.memset
+; CHECK: do_safepoint
+; CHECK: @foo
+  call void @llvm.memset.p1i8.i64(i8 addrspace(1)* %ptr, i8 0, i64 24, i32 8, i1 false)
+  call void @foo()
+  ret void
+}
+
+declare void @foo()
+declare void @llvm.memset.p1i8.i64(i8 addrspace(1)*, i8, i64, i32, i1)
+
+declare void @do_safepoint()
+define void @gc.safepoint_poll() {
+  call void @do_safepoint()
+  ret void
+}