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

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 problem ergonomically, 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.

Reviewers: reames, swaroop.sridhar

Subscribers: llvm-commits

Differential Revision: http://reviews.llvm.org/D11550

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@243502 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/docs/Statepoints.rst b/docs/Statepoints.rst
index 0d91f1b16b08630d41c44e58c5aba2e3279c0117..2b01374a9ec5cd3bf9d218be56362e61fec96b43 100644 (file)
--- a/docs/Statepoints.rst
+++ b/docs/Statepoints.rst
@@ -331,14 +331,16 @@ the user will patch over the 'num patch bytes' bytes of nops with a
 calling sequence specific to their runtime before executing the
 generated machine code.  There are no guarantees with respect to the
 alignment of the nop sequence.  Unlike :doc:`StackMaps` statepoints do
 calling sequence specific to their runtime before executing the
 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.
+not have a concept of shadow bytes.  Note that semantically the
+statepoint still represents a call or invoke to 'target', and the nop
+sequence after patching is expected to represent an operation
+equivalent to a call or invoke to 'target'.

 
 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
 
 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
 
 The '#call args' operand is the number of arguments to the actual
 call.  It must exactly match the number of arguments passed in the

diff --git a/lib/CodeGen/SelectionDAG/StatepointLowering.cpp b/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
index 34688df4765b688fc52295778b2835ac6adfdb9a..c62428d308e45b1d24ef90898c1ed306b8ec200a 100644 (file)
--- a/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
+++ b/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
@@ -289,7 +289,23 @@ lowerCallFromStatepoint(ImmutableStatepoint ISP, MachineBasicBlock *LandingPad,
 
   ImmutableCallSite CS(ISP.getCallSite());
 
 
   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!");
 
   assert(CS.getCallingConv() != CallingConv::AnyReg &&
          "anyregcc is not supported on statepoints!");

diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp
index 12e4f4ed0565ac15f0ad4a74f07c31683344e60c..2135dafef8d94414bc38a9ac91a16b3c12c84db2 100644 (file)
--- a/lib/IR/Verifier.cpp
+++ b/lib/IR/Verifier.cpp
@@ -1544,12 +1544,6 @@ void Verifier::VerifyStatepoint(ImmutableCallSite CS) {
          "gc.statepoint callee must be of function pointer type", &CI, Target);
   FunctionType *TargetFuncType = cast<FunctionType>(PT->getElementType());
 
          "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 "
   const Value *NumCallArgsV = CS.getArgument(3);
   Assert(isa<ConstantInt>(NumCallArgsV),
          "gc.statepoint number of arguments to underlying call "

diff --git a/lib/Transforms/Scalar/PlaceSafepoints.cpp b/lib/Transforms/Scalar/PlaceSafepoints.cpp
index 366301ad731a5779d96246f2d174c349639436f8..098e6eff15f7602b352dae0581c0c8fe7f4fca0c 100644 (file)
--- a/lib/Transforms/Scalar/PlaceSafepoints.cpp
+++ b/lib/Transforms/Scalar/PlaceSafepoints.cpp
@@ -917,15 +917,10 @@ static Value *ReplaceWithStatepoint(const CallSite &CS, /* to replace */
       CS.getInstruction()->getContext(), AttributeSet::FunctionIndex,
       AttrsToRemove);
 
       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(
   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());
         makeArrayRef(CS.arg_begin(), CS.arg_end()), None, None,
         "safepoint_token");
     Call->setTailCall(ToReplace->isTailCall());


@@ -951,7 +946,7 @@ static Value *ReplaceWithStatepoint(const CallSite &CS, /* to replace */
     // original block.
     Builder.SetInsertPoint(ToReplace->getParent());
     InvokeInst *Invoke = Builder.CreateGCStatepointInvoke(
     // 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");
 
         ToReplace->getUnwindDest(), makeArrayRef(CS.arg_begin(), CS.arg_end()),
         None, None, "safepoint_token");
 

diff --git a/test/Transforms/PlaceSafepoints/patchable-statepoints.ll b/test/Transforms/PlaceSafepoints/patchable-statepoints.ll
index 9387f42bf0abc252355043432ce030219f9f02c7..fd653174a6a453695357ae13ca1982950d4dd929 100644 (file)
--- a/test/Transforms/PlaceSafepoints/patchable-statepoints.ll
+++ b/test/Transforms/PlaceSafepoints/patchable-statepoints.ll
@@ -22,7 +22,7 @@ define void @test_num_patch_bytes() gc "statepoint-example" personality i32 ()*
 ; CHECK-LABEL: @test_num_patch_bytes(
 entry:
 ; CHECK-LABEL: entry:
 ; 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:
   invoke void @f()  "statepoint-num-patch-bytes"="99" to label %normal_return unwind label %exceptional_return
 
 normal_return:

diff --git a/test/Verifier/invalid-patchable-statepoint.ll b/test/Verifier/invalid-patchable-statepoint.ll
deleted file mode 100644 (file)
index 4783fa5..0000000
--- a/test/Verifier/invalid-patchable-statepoint.ll
+++ /dev/null
@@ -1,14 +0,0 @@
-; 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()