[Statepoints] Let patchable statepoints have a symbolic call target.

[sanjoy]

Summary

As added initially, statepoints required their call targets to be a
constant pointer null if `numPatchBytes` was non-zero. This turns out
to be a ergonomic problem, since there is no way to mark patchable
statepoints as calling a readable symbolic value.

This change remove the restriction of requiring `null` call targets
for patchable statepoints, and changes PlaceSafepoints to maintain the
symbolic call target through its transformation.

[sanjoy]

• clarify doc

docs/Statepoints.rst

 generated machine code.  There are no guarantees with respect to the
 alignment of the nop sequence.  Unlike :doc:`StackMaps` statepoints do
 not have a concept of shadow bytes.
 
 The 'target' operand is the function actually being called.  The
 target can be specified as either a symbolic LLVM function, or as an
 arbitrary Value of appropriate function type.  Note that the function
 type must match the signature of the callee and the types of the 'call
-parameters' arguments.  If 'num patch bytes' is non-zero then 'target'
-has to be the constant pointer null of the appropriate function type.
+parameters' arguments.
 
 The '#call args' operand is the number of arguments to the actual
 call.  It must exactly match the number of arguments passed in the
 'call parameters' variable length section.
 
 The 'flags' operand is used to specify extra information about the
 statepoint. This is currently only used to mark certain statepoints
 as GC transitions. This operand is a 64-bit integer with the following

lib/CodeGen/SelectionDAG/StatepointLowering.cpp

 /// reference lowered call result
 static SDNode *
 lowerCallFromStatepoint(ImmutableStatepoint ISP, MachineBasicBlock *LandingPad,
                         SelectionDAGBuilder &Builder,
                         SmallVectorImpl<SDValue> &PendingExports) {
 
   ImmutableCallSite CS(ISP.getCallSite());
 
-  SDValue ActualCallee = Builder.getValue(ISP.getCalledValue());
+  SDValue ActualCallee;
+
+  if (ISP.getNumPatchBytes() > 0) {
+    // If we've been asked to emit a nop sequence instead of a call instruction
+    // for this statepoint then don't lower the call target, but use a constant
+    // `null` instead.  Not lowering the call target lets statepoint clients get
+    // away without providing a physical address for the symbolic call target at
+    // link time.
+
+    const auto &TLI = Builder.DAG.getTargetLoweringInfo();
+    const auto &DL = Builder.DAG.getDataLayout();
+
+    unsigned AS = ISP.getCalledValue()->getType()->getPointerAddressSpace();
+    ActualCallee = Builder.DAG.getConstant(0, Builder.getCurSDLoc(),
+                                           TLI.getPointerTy(DL, AS));
+  } else
+    ActualCallee = Builder.getValue(ISP.getCalledValue());
 
   assert(CS.getCallingConv() != CallingConv::AnyReg &&
          "anyregcc is not supported on statepoints!");
 
   Type *DefTy = ISP.getActualReturnType();
   bool HasDef = !DefTy->isVoidTy();
 
   SDValue ReturnValue, CallEndVal;

lib/IR/Verifier.cpp

Assert(NumPatchBytes >= 0, "gc.statepoint number of patchable bytes must be "
          &CI);
 
   const Value *Target = CS.getArgument(2);
   const PointerType *PT = dyn_cast<PointerType>(Target->getType());
   Assert(PT && PT->getElementType()->isFunctionTy(),
          "gc.statepoint callee must be of function pointer type", &CI, Target);
   FunctionType *TargetFuncType = cast<FunctionType>(PT->getElementType());
 
-  if (NumPatchBytes)
-    Assert(isa<ConstantPointerNull>(Target->stripPointerCasts()),
-           "gc.statepoint must have null as call target if number of patchable "
-           "bytes is non zero",
-           &CI);
-
   const Value *NumCallArgsV = CS.getArgument(3);
   Assert(isa<ConstantInt>(NumCallArgsV),
          "gc.statepoint number of arguments to underlying call "
          "must be constant integer",
          &CI);
   const int NumCallArgs = cast<ConstantInt>(NumCallArgsV)->getZExtValue();
   Assert(NumCallArgs >= 0,
          "gc.statepoint number of arguments to underlying call "

lib/Transforms/Scalar/PlaceSafepoints.cpp

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(
-        ID, NumPatchBytes, StatepointTarget,
+        ID, NumPatchBytes, CS.getCalledValue(),
         makeArrayRef(CS.arg_begin(), CS.arg_end()), None, None,
         "safepoint_token");
     Call->setTailCall(ToReplace->isTailCall());
     Call->setCallingConv(ToReplace->getCallingConv());
 
     // 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.
static Value *ReplaceWithStatepoint(const CallSite &CS, /* to replace */
   } 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(
-        ID, NumPatchBytes, StatepointTarget, ToReplace->getNormalDest(),
+        ID, NumPatchBytes, CS.getCalledValue(), ToReplace->getNormalDest(),
         ToReplace->getUnwindDest(), makeArrayRef(CS.arg_begin(), CS.arg_end()),
         None, None, "safepoint_token");
 
     Invoke->setCallingConv(ToReplace->getCallingConv());
 
     // 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.

test/Transforms/PlaceSafepoints/patchable-statepoints.ll

exceptional_return:
   %landing_pad4 = landingpad {i8*, i32} cleanup
   ret void
 }
 
 define void @test_num_patch_bytes() gc "statepoint-example" personality i32 ()* @personality_function {
 ; 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,
+; CHECK: invoke i32 (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 2882400000, i32 99, void ()* @f,
   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} cleanup
   ret void

test/Verifier/invalid-patchable-statepoint.ll

-; RUN: not opt -verify 2>&1 < %s | FileCheck %s
-
-; CHECK: gc.statepoint must have null as call target if number of patchable bytes is non zero
-
-define i1 @invalid_patchable_statepoint() gc "statepoint-example" {
-entry:
-  %safepoint_token = tail call i32 (i64, i32, i1 ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_i1f(i64 0, i32 3, i1 ()* @func, i32 0, i32 0, i32 0, i32 0)
-  %call1 = call i1 @llvm.experimental.gc.result.i1(i32 %safepoint_token)
-  ret i1 %call1
-}
-
-declare i32 @llvm.experimental.gc.statepoint.p0f_i1f(i64, i32, i1 ()*, i32, i32, ...)
-declare i1 @llvm.experimental.gc.result.i1(i32)
-declare i1 @func()