[PlaceSafepoints] New attributes for patchable statepoints.

[sanjoy]

Summary

This patch teaches the PlaceSafepoints pass about two CallSite
function attributes:

• "statepoint-id": if the string value of this attribute can be parsed as an integer, then it is propagated to the ID parameter of the statepoint created.
• "statepoint-num-patch-bytes": if the string value of this attribute can be parsed as an integer, then it is propagated to the num patch bytes parameter of the statepoint created.

This change intentionally does not assert on a malformed value for these
attributes, given that they're not "official" attributes.

[pgavlin]

It would be nice to have documentation for these attributes somewhere.

lib/Transforms/Scalar/PlaceSafepoints.cpp

908–911 ↗

(On Diff #25663)

Are these attributes propagated by PlaceSafepoints? If so, should they be dropped instead? I don't necessarily see any reason to drop them; just wondering if this was considered.

[sanjoy]

lib/Transforms/Scalar/PlaceSafepoints.cpp

908–911 ↗

(On Diff #25663)

I think dropping them is a good idea, if nothing else, it makes the intent more obvious.

[sanjoy]

• address review: drop attributes once they're no longer needed.
• address review: docs

[pgavlin]

LGTM. Thanks for the updates.

llvm/trunk/docs/Statepoints.rst

 execution of a poll under worst case conditions such as heavy system paging.
 
 The implementation of a safepoint poll action is specified by looking up a 
 function of the name ``gc.safepoint_poll`` in the containing Module.  The body
 of this function is inserted at each poll site desired.  While calls or invokes
 inside this method are transformed to a ``gc.statepoints``, recursive poll 
 insertion is not performed.
 
+By default PlaceSafepoints passes in ``0xABCDEF00`` as the statepoint
+ID and ``0`` as the number of patchable bytes to the newly constructed
+``gc.statepoint``.  These values can be configured on a per-callsite
+basis using the attributes ``"statepoint-id"`` and
+``"statepoint-num-patch-bytes"``.  If a call site is marked with a
+``"statepoint-id"`` function attribute and its value is a positive
+integer (represented as a string), then that value is used as the ID
+of the newly constructed ``gc.statepoint``.  If a call site is marked
+with a ``"statepoint-num-patch-bytes"`` function attribute and its
+value is a positive integer, then that value is used as the 'num patch
+bytes' parameter of the newly constructed ``gc.statepoint``.  The
+``"statepoint-id"`` and ``"statepoint-num-patch-bytes"`` attributes
+are not propagated to the ``gc.statepoint`` call or invoke if they
+could be successfully parsed.
+
 If you are scheduling the RewriteStatepointsForGC pass late in the pass order,
 you should probably schedule this pass immediately before it.  The exception 
 would be if you need to preserve abstract frame information (e.g. for
 deoptimization or introspection) at safepoints.  In that case, ask on the 
 llvmdev mailing list for suggestions.
 
 
 Bugs and Enhancements

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

static Value *ReplaceWithStatepoint(const CallSite &CS, /* to replace */
   // be replacing a terminator.
   IRBuilder<> Builder(CS.getInstruction());
 
   // Note: The gc args are not filled in at this time, that's handled by
   // RewriteStatepointsForGC (which is currently under review).
 
   // Create the statepoint given all the arguments
   Instruction *Token = nullptr;
-  AttributeSet OriginalAttrs;
+
+  uint64_t ID;
+  uint32_t NumPatchBytes;
+
+  AttributeSet OriginalAttrs = CS.getAttributes();
+  Attribute AttrID =
+      OriginalAttrs.getAttribute(AttributeSet::FunctionIndex, "statepoint-id");
+  Attribute AttrNumPatchBytes = OriginalAttrs.getAttribute(
+      AttributeSet::FunctionIndex, "statepoint-num-patch-bytes");
+
+  AttrBuilder AttrsToRemove;
+  bool HasID = AttrID.isStringAttribute() &&
+               !AttrID.getValueAsString().getAsInteger(10, ID);
+
+  if (HasID)
+    AttrsToRemove.addAttribute("statepoint-id");
+  else
+    ID = 0xABCDEF00;
+
+  bool HasNumPatchBytes =
+      AttrNumPatchBytes.isStringAttribute() &&
+      !AttrNumPatchBytes.getValueAsString().getAsInteger(10, NumPatchBytes);
+
+  if (HasNumPatchBytes)
+    AttrsToRemove.addAttribute("statepoint-num-patch-bytes");
+  else
+    NumPatchBytes = 0;
+
+  OriginalAttrs = OriginalAttrs.removeAttributes(
+      CS.getInstruction()->getContext(), AttributeSet::FunctionIndex,
+      AttrsToRemove);
+
+  Value *StatepointTarget = NumPatchBytes == 0
+                                ? CS.getCalledValue()
+                                : ConstantPointerNull::get(cast<PointerType>(
+                                      CS.getCalledValue()->getType()));
 
   if (CS.isCall()) {
     CallInst *ToReplace = cast<CallInst>(CS.getInstruction());
     CallInst *Call = Builder.CreateGCStatepointCall(
-        0xABCDEF00, 0, CS.getCalledValue(), makeArrayRef(CS.arg_begin(), CS.arg_end()),
-        None, None, "safepoint_token");
+        ID, NumPatchBytes, StatepointTarget,
+        makeArrayRef(CS.arg_begin(), CS.arg_end()), None, None,
+        "safepoint_token");
     Call->setTailCall(ToReplace->isTailCall());
     Call->setCallingConv(ToReplace->getCallingConv());
 
-    // Before we have to worry about GC semantics, all attributes are legal
-    // TODO: handle param attributes
-    OriginalAttrs = ToReplace->getAttributes();
-
     // In case if we can handle this set of attributes - set up function
     // attributes directly on statepoint and return attributes later for
     // gc_result intrinsic.
     Call->setAttributes(OriginalAttrs.getFnAttributes());
 
     Token = Call;
 
     // Put the following gc_result and gc_relocate calls immediately after the
     // the old call (which we're about to delete).
     assert(ToReplace->getNextNode() && "not a terminator, must have next");
     Builder.SetInsertPoint(ToReplace->getNextNode());
     Builder.SetCurrentDebugLocation(ToReplace->getNextNode()->getDebugLoc());
   } else if (CS.isInvoke()) {
     InvokeInst *ToReplace = cast<InvokeInst>(CS.getInstruction());
 
     // Insert the new invoke into the old block.  We'll remove the old one in a
     // moment at which point this will become the new terminator for the
     // original block.
     Builder.SetInsertPoint(ToReplace->getParent());
     InvokeInst *Invoke = Builder.CreateGCStatepointInvoke(
-        0xABCDEF00, 0, CS.getCalledValue(), ToReplace->getNormalDest(),
+        ID, NumPatchBytes, StatepointTarget, ToReplace->getNormalDest(),
         ToReplace->getUnwindDest(), makeArrayRef(CS.arg_begin(), CS.arg_end()),
         Builder.getInt32(0), None, "safepoint_token");
 
-    // Currently we will fail on parameter attributes and on certain
-    // function attributes.
-    OriginalAttrs = ToReplace->getAttributes();
-
     // In case if we can handle this set of attributes - set up function
     // attributes directly on statepoint and return attributes later for
     // gc_result intrinsic.
     Invoke->setAttributes(OriginalAttrs.getFnAttributes());
 
     Token = Invoke;
 
     // We'll insert the gc.result into the normal block

llvm/trunk/test/Transforms/PlaceSafepoints/patchable-statepoints.ll

+; RUN: opt -place-safepoints -S < %s | FileCheck %s
+
+declare void @f()
+declare i32 @personality_function()
+
+define void @test_id() gc "statepoint-example" {
+; CHECK-LABEL: @test_id(
+entry:
+; CHECK-LABEL: entry:
+; CHECK: invoke i32 (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 100, i32 0, void ()* @f
+  invoke void @f()  "statepoint-id"="100" to label %normal_return unwind label %exceptional_return
+
+normal_return:
+  ret void
+
+exceptional_return:
+  %landing_pad4 = landingpad {i8*, i32} personality i32 ()* @personality_function cleanup
+  ret void
+}
+
+define void @test_num_patch_bytes() gc "statepoint-example" {
+; CHECK-LABEL: @test_num_patch_bytes(
+entry:
+; CHECK-LABEL: entry:
+; CHECK: invoke i32 (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 2882400000, i32 99, void ()* null,
+  invoke void @f()  "statepoint-num-patch-bytes"="99" to label %normal_return unwind label %exceptional_return
+
+normal_return:
+  ret void
+
+exceptional_return:
+  %landing_pad4 = landingpad {i8*, i32} personality i32 ()* @personality_function cleanup
+  ret void
+}
+
+declare void @do_safepoint()
+define void @gc.safepoint_poll() {
+entry:
+  call void @do_safepoint()
+  ret void
+}
+
+; CHECK-NOT: statepoint-id
+; CHECK-NOT: statepoint-num-patch_bytes