From 44d65eac432eaa3e926ec404a55e66f823cb0848 Mon Sep 17 00:00:00 2001 From: Sanjoy Das Date: Tue, 28 Jul 2015 23:50:30 +0000 Subject: [PATCH] [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 --- docs/Statepoints.rst | 8 +++++--- .../SelectionDAG/StatepointLowering.cpp | 18 +++++++++++++++++- lib/IR/Verifier.cpp | 6 ------ lib/Transforms/Scalar/PlaceSafepoints.cpp | 9 ++------- .../PlaceSafepoints/patchable-statepoints.ll | 2 +- test/Verifier/invalid-patchable-statepoint.ll | 14 -------------- 6 files changed, 25 insertions(+), 32 deletions(-) delete mode 100644 test/Verifier/invalid-patchable-statepoint.ll diff --git a/docs/Statepoints.rst b/docs/Statepoints.rst index 0d91f1b16b0..2b01374a9ec 100644 --- 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 -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 -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 diff --git a/lib/CodeGen/SelectionDAG/StatepointLowering.cpp b/lib/CodeGen/SelectionDAG/StatepointLowering.cpp index 34688df4765..c62428d308e 100644 --- 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()); - 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!"); diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp index 12e4f4ed056..2135dafef8d 100644 --- 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(PT->getElementType()); - if (NumPatchBytes) - Assert(isa(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(NumCallArgsV), "gc.statepoint number of arguments to underlying call " diff --git a/lib/Transforms/Scalar/PlaceSafepoints.cpp b/lib/Transforms/Scalar/PlaceSafepoints.cpp index 366301ad731..098e6eff15f 100644 --- 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); - Value *StatepointTarget = NumPatchBytes == 0 - ? CS.getCalledValue() - : ConstantPointerNull::get(cast( - CS.getCalledValue()->getType())); - if (CS.isCall()) { CallInst *ToReplace = cast(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()); @@ -951,7 +946,7 @@ static Value *ReplaceWithStatepoint(const CallSite &CS, /* to replace */ // 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"); diff --git a/test/Transforms/PlaceSafepoints/patchable-statepoints.ll b/test/Transforms/PlaceSafepoints/patchable-statepoints.ll index 9387f42bf0a..fd653174a6a 100644 --- 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: 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: diff --git a/test/Verifier/invalid-patchable-statepoint.ll b/test/Verifier/invalid-patchable-statepoint.ll deleted file mode 100644 index 4783fa57f8f..00000000000 --- 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() -- 2.34.1